-
Notifications
You must be signed in to change notification settings - Fork 135
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
[Subtask] Cli
method for blacklist update
#768
Comments
Cli
method for blacklist update
I'm Interested in this, could you assign it to me?Should I wait for 770 to be completed before starting? |
Of course, I assigned it to you. |
cc @Kwafoor #770 has been finished. You can go ahead. |
OK,I will finish as soon as possible. |
Almost done, just a little more checking and final touches left. I'll submit it in a few days. |
@Kwafoor We usually use rest api to control admin service. If we need add api, we would better add rest api. Could we keep consistent with other operation. You can refer to #684 |
I have implemented control admin service with reference to yarn through grpc, but I will refer to #684 to see how to achieve it through rest. |
### What changes were proposed in this pull request? I created a command called uniffle cli to update blacklist, the following is the case of using the command. ``` Usage: Optional -a,--admin <arg> This is an admin command that will print args. -c,--cli <arg> This is an client cli command that will print args. -h,--help Help for the Uniffle CLI. -rc,--refreshChecker This is an admin command that will refresh access checker.. -cc,--checkerClass <arg> This is an client cli command that will print args. -s,--help <arg> This is coordinator server host. -p,--port <arg> This is coordinator server port. ``` > Run the example-cli command uniffle admin-cli -rc -cc org.apache.uniffle.test.AccessClusterTest$MockedAccessChecker -s localhost -p 12345 ### Why are the changes needed? We use refreshChecker cli to update blacklist. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Unit test verification.
closed by #931 |
### What changes were proposed in this pull request? While reading the code, I found some minor issues, this PR will fix them. - Issue1: When using the `uniffle admin-cli` command without any options, it should print the help information. > before modification <img width="575" alt="image" src="https://github.com/apache/incubator-uniffle/assets/55643692/8324e9fa-8bf7-4b8c-bbd4-76795fa8e105"> > after modification <img width="660" alt="image" src="https://github.com/apache/incubator-uniffle/assets/55643692/8e3b53a8-faec-4d38-ae97-a420ba48c3fa"> - Issue2: The comment information is incorrect. ### Why are the changes needed? Improve code readability and enhance code functionality. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Test environment verification.
Code of Conduct
Search before asking
Describe the subtask
Currently, updates to the blacklist need to rely on the scheduling thread pool for updates, which is relatively inefficient. We can emulate the Hadoop method of updating the config and add clis for updates, which is more efficient.
Parent issue
#758
Are you willing to submit PR?
The text was updated successfully, but these errors were encountered: