Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Subtask] support schema pre event for Gravitino server #5547

Closed
Tracked by #5317
FANNG1 opened this issue Nov 12, 2024 · 4 comments · Fixed by #5559
Closed
Tracked by #5317

[Subtask] support schema pre event for Gravitino server #5547

FANNG1 opened this issue Nov 12, 2024 · 4 comments · Fixed by #5559
Assignees
@orenccl
Labels
0.8.0 Release v0.8.0 help wanted Extra attention is needed subtask Subtasks of umbrella issue

Comments

@FANNG1
Copy link
Contributor

FANNG1 commented Nov 12, 2024

Describe the subtask

  1. add corresponding SchemaPreEvent
  2. generate pre-event in SchemaEventDispatcher
  3. add test in TestSchemaEvent
  4. add document in gravitino-server-config.md

Please refer to the implemetation of table pre event in #5539

Parent issue

#5317
@FANNG1 FANNG1 added the subtask Subtasks of umbrella issue label Nov 12, 2024
@FANNG1 FANNG1 mentioned this issue Nov 12, 2024
Open
7 tasks
@FANNG1 FANNG1 added the help wanted Extra attention is needed label Nov 12, 2024
@orenccl
Copy link
Contributor

orenccl commented Nov 12, 2024

It has a reference. I think I’ll be able to take this one. Please assign it to me, and I’ll work on it within a few days.

@xunliu
Copy link
Member

xunliu commented Nov 12, 2024

@orenccl Thank you for your interesting in this issue.
I assigned it to you.

@orenccl
Copy link
Contributor

orenccl commented Nov 12, 2024 •
edited
Loading

Hi @FANNG1,

I noticed that in AlterTableEvent, this.tableChanges is set to tableChanges.clone(),
while in AlterTablePreEvent, this.tableChanges is assigned directly as tableChanges.
Should it also be tableChanges.clone() in AlterTablePreEvent?

Additionally, in gravitino-server-config.md, updateTablePreEvent should be updated to AlterTablePreEvent.

I've made these modifications in the PR; please let me know if this approach looks good to you.

@orenccl orenccl mentioned this issue Nov 12, 2024
Merged
@FANNG1
Copy link
Contributor Author

FANNG1 commented Nov 13, 2024

Good catch, @orenccl ! I commented related issues in the PR.

@FANNG1 FANNG1 added the 0.8.0 Release v0.8.0 label Nov 13, 2024
FANNG1 pushed a commit that referenced this issue Nov 13, 2024
@orenccl
[#5547] feat(core): support schema pre event for Gravitino server (#5559
3cd372f 
)

### What changes were proposed in this pull request?

add schema pre event to event listener

### Why are the changes needed?

Close: #5547

### Does this PR introduce _any_ user-facing change?

no

### How was this patch tested?

add Unit Test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@orenccl orenccl

Labels
0.8.0 Release v0.8.0 help wanted Extra attention is needed subtask Subtasks of umbrella issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[#5547] feat(core): support schema pre event for Gravitino server orenccl/gravitino
3 participants
@xunliu @FANNG1 @orenccl