gravitino icon indicating copy to clipboard operation
gravitino copied to clipboard

[#6814] feat(core): Support update aliases for model version.

Open Abyss-lord opened this issue 8 months ago • 6 comments

What changes were proposed in this pull request?

Support update aliases for model version.

  • [X] PR1: Add ModelVersionChange API interface, Implement the update alias logic in model catalog and JDBC backend logic, update related event.
  • [ ] PR2: Add REST endpoint to support model version change, add Java client and Python client for model version alias update.

Why are the changes needed?

Fix: #6814

Does this PR introduce any user-facing change?

no.

How was this patch tested?

local test. When an alias is updated, all old aliases are removed and all new aliases are inserted, keeping the version number unchanged

image

Abyss-lord avatar Apr 22 '25 09:04 Abyss-lord

Hi @jerryshao , could you please review this PR when you have time? I’d really appreciate your feedback.

Abyss-lord avatar Apr 23 '25 01:04 Abyss-lord

Hi @jerryshao , I've completed the code updates and would appreciate your review of the PR when you have a moment. Here's a summary of the commits:

  1. Refactor the UpdateAlias class.

Abyss-lord avatar Apr 27 '25 07:04 Abyss-lord

Can you please also update the doc in this PR?

jerryshao avatar Apr 28 '25 03:04 jerryshao

Can you please also update the doc in this PR?

@jerryshao Personally, I think it is appropriate to add in the next PR, this PR has not added the request related content.

Abyss-lord avatar Apr 28 '25 04:04 Abyss-lord

Hi @jerryshao , I've completed the code updates and would appreciate your review of the PR when you have a moment. Here's a summary of the commits:

  1. Add new test cases
  2. Fix typo issues.

Abyss-lord avatar Apr 28 '25 05:04 Abyss-lord

Hi @jerryshao , I've completed the code updates and would appreciate your review of the PR when you have a moment. Here's a summary of the commits:

  1. Fix NPE in static method of ModelVersionChange.updateAliases()
  2. Fix handle logic of ModelVersionChange.updateAliases(), If use remove and add the same alias, nothing happens.

Abyss-lord avatar Apr 29 '25 06:04 Abyss-lord

Generally LGTM, just one issue need to be fixed. @Abyss-lord

jerryshao avatar May 07 '25 08:05 jerryshao

Hi @jerryshao , I've completed the code updates and would appreciate your review of the PR when you have a moment. Here's a summary of the commits:

  1. Move Duplicate removal logic in server side.

Abyss-lord avatar May 08 '25 01:05 Abyss-lord

The first part is done, please do the followup things.

jerryshao avatar May 08 '25 03:05 jerryshao