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.

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

[#4105] improvement(core): Remove the logic of getValidRoles #4121

Merged
merged 2 commits into from
Jul 12, 2024

Conversation

jerqi
Copy link
Contributor

@jerqi jerqi commented Jul 10, 2024

What changes were proposed in this pull request?

Remove the logic of getValidRoles.

Why are the changes needed?

Fix: #4105

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Modify some test cases.

@jerqi
Copy link
Contributor Author

jerqi commented Jul 10, 2024

@jerryshao @yuqi1129 Could you help me review this pull request?

@jerqi jerqi force-pushed the ISSUE-4105 branch 2 times, most recently from d934a3c to be9cea6 Compare July 10, 2024 08:20
for (String role : userEntity.roleNames()) {
roleEntities.add(roleManager.getRole(metalake, role));
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest that we don't store the role name in user entity, only store the id, and we don't need to check role name, but the role id.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't store role name in the underlying relational database. It's necessary for us to assign the role names fields in the user entity. The updated user entity will be returned to end user. The users of requests need role names for human understanding.

@jerryshao jerryshao changed the title [#4105] improvement: Remove the logic of getValidRoles [#4105] improvement(core): Remove the logic of getValidRoles Jul 12, 2024
@jerryshao jerryshao merged commit d613544 into apache:main Jul 12, 2024
33 checks passed
@jerqi jerqi deleted the ISSUE-4105 branch July 12, 2024 08:27
jerryshao pushed a commit to jerryshao/gravitino that referenced this pull request Jul 16, 2024
…pache#4121)

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

Remove the logic of getValidRoles.

### Why are the changes needed?

Fix: apache#4105 

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

### How was this patch tested?
Modify some test cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improvement] Remove the logic of getValidRoles
2 participants