[#6078] feat(core): Support model event to Gravitino server #6129

Abyss-lord · 2025-01-07T07:21:28Z

What changes were proposed in this pull request?

Support model event to Gravitino server. Add list, get, create and delete events to both model and model version.
test case will add after EventDispatcher merged.

Event	OperationType	ModelCatalog
`RegisterModelEvent`	`REGISTER_MODEL`	`registerModel`
`GetModelEvent`	`LOAD_MODEL`	`getModel`
`DropModelEvent`	`DROP_MODEL`	`deleteModel`
`ListModelEvent`	`LIST_MODEL`	`listModels`
`LinkModelVersionEvent`	`LINK_MODEL_VERSION`	`linkModelVersion`
`GetModelVersionEvent`	`GET_MODEL_VERSION`	`getModelVersion`
`DropModelVersionEvent`	`DROP_MODEL_VERSION`	`deleteModelVersion`
`ListModelVersionsEvent`	`LIST_MODEL_VERSIONS`	`listModelVersions`

Why are the changes needed?

Fix: #6078

Does this PR introduce any user-facing change?

no

How was this patch tested?

local test.

tengqm · 2025-01-08T00:03:12Z

core/src/main/java/org/apache/gravitino/listener/api/event/GetModelVersionEvent.java

+  public GetModelVersionEvent(
+      String user, NameIdentifier identifier, ModelInfo getModelVersionInfo) {
+    super(user, identifier);
+    GetModelVersionInfo = getModelVersionInfo;


Suggested change

GetModelVersionInfo = getModelVersionInfo;

this.info = getModelVersionInfo;

tengqm · 2025-01-08T00:04:56Z

core/src/main/java/org/apache/gravitino/listener/api/info/ModelInfo.java

+   * @param model the model to expose information for.
+   * @param modelVersion the versions of the model.
+   */
+  public ModelInfo(Model model, ModelVersion[] modelVersion) {


Suggested change

public ModelInfo(Model model, ModelVersion[] modelVersion) {

public ModelInfo(Model model, ModelVersion[] modelVersions) {

Support model event to Gravitino server. Add list, get, create and delete events to both model and model version.

Fix some class comments.

Fix some error.

FANNG1 · 2025-01-09T07:01:16Z

Will review the PR these days.

…on details page (apache#6167) ### What changes were proposed in this pull request? Reload tree data after refreshing the version details page <img width="1352" alt="image" src="https://github.com/user-attachments/assets/80bc7370-0052-4be8-9b6d-efbe6d97e945" /> ### Why are the changes needed? N/A Fix: apache#6166 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? manually

…apache#6159) ### What changes were proposed in this pull request? 1. Refactor catalog commands and Base class in Gavitino CLI. 2. Add test case. ### Why are the changes needed? Fix: apache#6143 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? local test.

…pache#6161) ### What changes were proposed in this pull request? Refactor table commands in Gavitino CLI and Base class. ### Why are the changes needed? Fix: apache#6145 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? local test

…pache#6162) ### What changes were proposed in this pull request? Refactor model commands and Base class in Gavitino CLI. ### Why are the changes needed? Fix: apache#6148 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? local test

…eset catalog with cloud storage (apache#5974) ### What changes were proposed in this pull request? Support dynamic credential in obtaining cloud storage fileset. ### Why are the changes needed? Static key are not very safe, we need to optimize it. Fix: apache#5973 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? ITs

orenccl

I noticed that this PR includes commits from other PRs. It seems like those are unrelated to the feature in this PR. Could you please clean up the commit history to ensure only relevant changes are included?

### What changes were proposed in this pull request? Added LICENSE and NOTICE file for the Trino connector. ### Why are the changes needed? to comply with ASF policy Fix: apache#6173 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Tested locally.

### What changes were proposed in this pull request? Update year as we are going to make a new release. ### Why are the changes needed? ASF/legal policy. Fix: #N/A ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? N/A

Bumps [nanoid](https://github.com/ai/nanoid) from 3.3.7 to 3.3.8. <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/ai/nanoid/blob/main/CHANGELOG.md">nanoid's changelog</a>.</em></p> <blockquote> <h2>3.3.8</h2> <ul> <li>Fixed a way to break Nano ID by passing non-integer size (by <a href="https://github.com/myndzi"><code>@myndzi</code></a>).</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/ai/nanoid/commit/3044cd5e73f4cf31795f61f6e6b961c8c0a5c744"><code>3044cd5</code></a> Release 3.3.8 version</li> <li><a href="https://github.com/ai/nanoid/commit/4fe34959c34e5b3573889ed4f24fe91d1d3e7231"><code>4fe3495</code></a> Update size limit</li> <li><a href="https://github.com/ai/nanoid/commit/d643045f40d6dc8afa000a644d857da1436ed08c"><code>d643045</code></a> Fix pool pollution, infinite loop (<a href="https://redirect.github.com/ai/nanoid/issues/510">#510</a>)</li> <li>See full diff in <a href="https://github.com/ai/nanoid/compare/3.3.7...3.3.8">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=nanoid&package-manager=npm_and_yarn&previous-version=3.3.7&new-version=3.3.8)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/apache/gravitino/network/alerts). </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…pache#6174) ### What changes were proposed in this pull request? Refactor topic commands in Gavitino CLI ### Why are the changes needed? Fix: apache#6146 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? local test.

…pache#6175) ### What changes were proposed in this pull request? Refactor group commands in Gavitino CLI. ### Why are the changes needed? Fix: apache#6151 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? local test.

…ages for Iceberg REST server (apache#6168) ### What changes were proposed in this pull request? 1. use Gravitino cloud jar without hadoop packages for Iceberg REST server credential vending in test and document 2. For OSS, use Gravitino Aliyun bundle jar in test and docker image because Iceberg doesn't provide Iceberg Aliyun bundle jar ### Why are the changes needed? Fix: apache#6165 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? test S3 OSS GCS ADLS Iceberg REST test in local

…operations (apache#6141) ### What changes were proposed in this pull request? 1. for resource path like `a/b`, add "a", "a/", "a/b" read permission for GCS connector 2. replace `storage.legacyBucketReader` with `storage.insightsCollectorService`, because `storage.legacyBucketReader` provides extra list permission for the bucket. ### Why are the changes needed? Fix: apache#6054 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? Iceberg GCS IT and fileset GCS credential IT

…apache#6115) ### What changes were proposed in this pull request? Add document for chain authorization plugin ### Why are the changes needed? Fix: apache#6110 ### Does this PR introduce _any_ user-facing change? Just document. ### How was this patch tested? No need. --------- Co-authored-by: Xun <[email protected]> Co-authored-by: Qiming Teng <[email protected]>

…apache#6178) ### What changes were proposed in this pull request? Refactor schema commands in Gravitino CLI. ### Why are the changes needed? Fix: apache#6144 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Local test

### What changes were proposed in this pull request? Remove the unused protobuf dependency. ### Why are the changes needed? Since we already removed the KV storage support, so protobuf dependency is not required any more. Fix: apache#6184 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing tests.

…e#6170) ### What changes were proposed in this pull request? Break up role commands in Gravitino command line class ### Why are the changes needed? For readability and maintainability. ### Fix: apache#6153 ### Does this PR introduce any user-facing change? None. ### How was this patch tested? Tested locally.

Abyss-lord · 2025-01-11T02:20:40Z

I noticed that this PR includes commits from other PRs. It seems like those are unrelated to the feature in this PR. Could you please clean up the commit history to ensure only relevant changes are included?

Hi @orenccl , I’ve finished updating the code. Please take a look at the PR again when you have time.

orenccl

According to #6078, I think we also need to generate event in ModelEventDispatcher and add unit test.

I didn’t see them included in this PR. Could you check?

Abyss-lord · 2025-01-13T03:24:15Z

According to #6078, I think we also need to generate event in ModelEventDispatcher and add unit test.

I didn’t see them included in this PR. Could you check?

@orenccl Thank you! This PR primarily aims to ensure that my design and implementation approach is correct. because this feature is split into 4 issue, If there are no errors, I will add test cases in #6075, WDYT

orenccl · 2025-01-13T03:40:52Z

In my opinion, I would prefer to handle #6075 and #6078 in the same PR.
But I am fine with current approach.

Currently, I am busy with my family as my father is in the hospital's intensive care unit (ICU).
Therefore, I may not be able to respond or review promptly.

I can only perform some simple checks for now. Please reach out to others for help with the review. Thank you!

orenccl · 2025-01-13T03:48:31Z

If you want to handle them separately, please refer to PR #5847 and #5998. I believe these PRs demonstrate how to properly split them into two separate PRs.

And I think we should handle #6075 first, and then handle #6078.

@FANNG1 WDYT?

Abyss-lord · 2025-01-13T03:54:55Z

If you want to handle them separately, please refer to PR #5847 and #5998. I believe these PRs demonstrate how to properly split them into two separate PRs.

And I think we should handle #6075 first, and then handle #6078.

@orenccl Thank you for sharing your thoughts . I appreciate you taking the time to provide feedback despite everything you're going through.Wishing your father a smooth and speedy recovery. I will add ModelEventDispatcher and unit test in this pr.

Take care!!

xunliu requested a review from FANNG1 January 7, 2025 07:33

tengqm reviewed Jan 8, 2025

View reviewed changes

Abyss-lord added 3 commits January 9, 2025 14:29

[apache#6078] feat(core): Support model event to Gravitino server

4b5e95a

Support model event to Gravitino server. Add list, get, create and delete events to both model and model version.

[apache#6078] feat(core): Support model event to Gravitino server

9098335

Fix some class comments.

[apache#6078] feat(core): Support model event to Gravitino server

cd86b6a

Fix some error.

Abyss-lord force-pushed the model-event branch from 799df18 to cd86b6a Compare January 9, 2025 06:30

LauraXia123 and others added 5 commits January 10, 2025 10:25

orenccl suggested changes Jan 10, 2025

View reviewed changes

justinmclean and others added 12 commits January 11, 2025 10:16

Merge branch 'main' into model-event

dc3444d

Abyss-lord requested a review from orenccl January 11, 2025 02:20

orenccl reviewed Jan 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#6078] feat(core): Support model event to Gravitino server #6129

[#6078] feat(core): Support model event to Gravitino server #6129

Abyss-lord commented Jan 7, 2025

tengqm Jan 8, 2025

tengqm Jan 8, 2025

FANNG1 commented Jan 9, 2025

orenccl left a comment

Abyss-lord commented Jan 11, 2025 •

edited

Loading

orenccl left a comment •

edited

Loading

Abyss-lord commented Jan 13, 2025

orenccl commented Jan 13, 2025 •

edited

Loading

orenccl commented Jan 13, 2025 •

edited

Loading

Abyss-lord commented Jan 13, 2025

	GetModelVersionInfo = getModelVersionInfo;
	this.info = getModelVersionInfo;

	public ModelInfo(Model model, ModelVersion[] modelVersion) {
	public ModelInfo(Model model, ModelVersion[] modelVersions) {

[#6078] feat(core): Support model event to Gravitino server #6129

Are you sure you want to change the base?

[#6078] feat(core): Support model event to Gravitino server #6129

Conversation

Abyss-lord commented Jan 7, 2025

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

tengqm Jan 8, 2025

Choose a reason for hiding this comment

tengqm Jan 8, 2025

Choose a reason for hiding this comment

FANNG1 commented Jan 9, 2025

orenccl left a comment

Choose a reason for hiding this comment

Abyss-lord commented Jan 11, 2025 • edited Loading

orenccl left a comment • edited Loading

Choose a reason for hiding this comment

Abyss-lord commented Jan 13, 2025

orenccl commented Jan 13, 2025 • edited Loading

orenccl commented Jan 13, 2025 • edited Loading

Abyss-lord commented Jan 13, 2025

Abyss-lord commented Jan 11, 2025 •

edited

Loading

orenccl left a comment •

edited

Loading

orenccl commented Jan 13, 2025 •

edited

Loading

orenccl commented Jan 13, 2025 •

edited

Loading