-
Notifications
You must be signed in to change notification settings - Fork 392
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.
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
[#2238] feat(server): Add the operations for the user #2733
Conversation
This reverts commit d6cef11.
() -> { | ||
NameIdentifier ident = ofUser(metalake, user); | ||
boolean removed = | ||
TreeLockUtils.doWithTreeLock( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yuqi1129 Could you help me review the use of lock?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part maybe changed after #2732 , let's wait for a while.
LGTM, @yuqi1129 would you please double the usage of tree lock here? |
No problem was found here, so it's okay for me with the changes. |
OK, I think we can change the whole lock mechanism in #2732 if necessary, we don't have to block this PR. |
What changes were proposed in this pull request?
Add the operations for the user.
Why are the changes needed?
Fix: #2238
Does this PR introduce any user-facing change?
Yes, I will add the open api and the document in the later pr.
How was this patch tested?
Add a new UT.