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

[Enhancement] (nereids)implement showRolesCommand in nereids #43118

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

Vallishp
Copy link
Contributor

@Vallishp Vallishp commented Nov 1, 2024

What problem does this PR solve?

Support show roles in nereids.

Issue Number: close #42768

Related PR: #xxx

Problem Summary:

Check List (For Committer)

  • Test

    • [*] Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No colde files have been changed.
      • Other reason
        2024-11-08 03:37:22.042 INFO [suite-thread-1] (test_nereids_role.groovy:64) - roles_drop: [[admin, null, 'admin'@'%', Admin_priv, null, null, null, null, null, null, null, null, null], [operator, null, 'root'@'%', Node_priv,Admin_priv, null, null, null, null, null, null, null, null, null]]
        2024-11-08 03:37:22.100 INFO [suite-thread-1] (ScriptContext.groovy:120) - Run test_nereids_role in /root/doris/workspace/doris/regression-test/suites/nereids_p0/ddl/account/test_nereids_role.groovy succeed
        2024-11-08 03:37:52.786 INFO [main] (RegressionTest.groovy:281) - Start to run single scripts
        2024-11-08 03:39:04.628 INFO [main] (RegressionTest.groovy:410) - Success suites:
        /root/doris/workspace/doris/regression-test/suites/nereids_p0/ddl/account/test_nereids_role.groovy: group=default,p0, name=test_nereids_role
        2024-11-08 03:39:04.629 INFO [main] (RegressionTest.groovy:492) - All suites success.

| _ \ / \ / / || | _
| |
) / _ \ _
_
| | | | | |
| / ___ \ ) |) | || |
| |
|
| /
/ __
//|_____|/

2024-11-08 03:39:04.629 INFO [main] (RegressionTest.groovy:440) - Test 1 suites, failed 0 suites, fatal 0 scripts, skipped 0 scripts
2024-11-08 03:39:04.629 INFO [main] (RegressionTest.groovy:126) - Test finished

  • Behavior changed:

    • No. No change.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.
  • Release note

    None

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@Vallishp
Copy link
Contributor Author

Vallishp commented Nov 1, 2024

run buildall

}

List<List<String>> infos = Env.getCurrentEnv().getAuth().getRoleInfo();
ShowResultSet resultSet = new ShowResultSet(getMetaData(), infos);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove getMetaData() method, use META_DATA directly

Copy link
Contributor

github-actions bot commented Nov 6, 2024

PR approved by anyone and no changes requested.

/**
* show roles command
*/
public class ShowRolesCommand extends Command implements NoForward {
Copy link
Contributor

Choose a reason for hiding this comment

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

The show command related features have some common logical need to be sorted out. I am working on the common part and the pr is #43271 . You can take it as a reference, the most important part is all show commands should inherent from ShowCommand and override the doRun method to do the real job.(you can move the code in run() function to doRun()) Sorry for the late change, you might have to wait the pr is merged and update all other show command prs accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. sure thanks. will handle all show related PR after merge of #43271

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@starocean999 i have updated as per your comments. Please help to review again. after that i will update all other show command PRs accordingly. thank you

@Vallishp
Copy link
Contributor Author

Vallishp commented Nov 8, 2024

run buildall

@Vallishp
Copy link
Contributor Author

run buildall

@Vallishp
Copy link
Contributor Author

run p0

Copy link
Contributor

@starocean999 starocean999 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 12, 2024
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@starocean999 starocean999 merged commit c2ac2cb into apache:master Nov 12, 2024
25 of 26 checks passed
zzzxl1993 pushed a commit to zzzxl1993/doris that referenced this pull request Nov 12, 2024
py023 pushed a commit to py023/doris that referenced this pull request Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] (nereids)implement showRolesCommand in nereids
4 participants