tengqm commented on code in PR #5618:
URL: https://github.com/apache/gravitino/pull/5618#discussion_r1851209776
##########
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java:
##########
@@ -42,10 +42,12 @@ public class GravitinoOptions {
public static final String TAG = "tag";
public static final String ROLE = "role";
public static final String AUDIT = "audit";
- public static final String INDEX = "index";
public static final String FORCE = "force";
+ public static final String SIMPLE = "simple";
+ public static final String LOGIN = "login";
+ public static final String INDEX = "index";
public static final String DISTRIBUTION = "distribution";
- public static final String Partition = "partition";
+ public static final String PARTITION = "partition";
Review Comment:
This long list of constants has to be refactored for maintainability.
##########
clients/cli/src/main/java/org/apache/gravitino/cli/commands/AddRoleToUser.java:
##########
@@ -38,13 +38,21 @@ public class AddRoleToUser extends Command {
*
* @param url The URL of the Gravitino server.
* @param ignoreVersions If true don't check the client/server versions
match.
+ * @param authentication Authentication type i.e. "simple"
+ * @param userName User name for simple authentication.
* @param metalake The name of the metalake.
* @param user The name of the user.
* @param role The name of the role.
*/
public AddRoleToUser(
- String url, boolean ignoreVersions, String metalake, String user, String
role) {
- super(url, ignoreVersions);
+ String url,
+ boolean ignoreVersions,
+ String authentication,
Review Comment:
```suggestion
String authType,
```
##########
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java:
##########
@@ -64,9 +66,11 @@ public Options options() {
options.addOption(createArgOption("m", METALAKE, "metalake name"));
options.addOption(createSimpleOption("i", IGNORE, "ignore client/sever
version check"));
options.addOption(createSimpleOption("a", AUDIT, "display audit
information"));
+ options.addOption(createSimpleOption(null, SIMPLE, "simple
authentication"));
+ options.addOption(createArgOption(null, LOGIN, "user name"));
options.addOption(createSimpleOption("x", INDEX, "Display index
infromation"));
options.addOption(createSimpleOption("d", DISTRIBUTION, "Display
distribution information"));
- options.addOption(createSimpleOption("p", Partition, "Display partition
information"));
+ options.addOption(createSimpleOption(null, PARTITION, "Display partition
information"));
Review Comment:
Some help messages start with a capital letter, others don't ...
Need to be consistent across all options, IMO.
##########
clients/cli/src/main/java/org/apache/gravitino/cli/commands/AddRoleToGroup.java:
##########
@@ -38,13 +38,21 @@ public class AddRoleToGroup extends Command {
*
* @param url The URL of the Gravitino server.
* @param ignoreVersions If true don't check the client/server versions
match.
+ * @param authentication Authentication type i.e. "simple"
+ * @param userName User name for simple authentication.
* @param metalake The name of the metalake.
* @param group The name of the group.
* @param role The name of the role.
*/
public AddRoleToGroup(
- String url, boolean ignoreVersions, String metalake, String group,
String role) {
- super(url, ignoreVersions);
+ String url,
+ boolean ignoreVersions,
+ String authentication,
Review Comment:
```suggestion
String authType,
```
##########
clients/cli/src/main/java/org/apache/gravitino/cli/commands/AuditCommand.java:
##########
@@ -25,9 +25,11 @@ public abstract class AuditCommand extends Command {
/**
* @param url The URL of the Gravitino server.
* @param ignoreVersions If true don't check the client/server versions
match.
+ * @param authentication Authentication type i.e. "simple"
+ * @param userName User name for simple authentication.
*/
- public AuditCommand(String url, boolean ignoreVersions) {
- super(url, ignoreVersions);
+ public AuditCommand(String url, boolean ignoreVersions, String
authentication, String userName) {
+ super(url, ignoreVersions, authentication, userName);
Review Comment:
Same as above, please consider a thorough check.
##########
docs/cli.md:
##########
@@ -522,51 +533,15 @@ gcli tag update --tag tagA --rename newTag
gcli tag update --tag tagA --comment "new comment"
```
-### Role commands
+### Authentication
-#### Display role details
+#### Simple authentication
```bash
-gcli role details --role admin
-```
-
-#### List all roles
-
-```bash
-gcli role list
-```
-
-#### Create a role
-
-```bash
-gcli role create --role admin
-```
-
-#### Delete a role
+gcli <normal command> --simple
-```bash
-gcli role delete --role admin
-```
-
-#### Add a role to a user
-
-```bash
-gcli user grant --user new_user --role admin
-```
-
-#### Remove a role from a user
-
-```bash
-gcli user revoke --user new_user --role admin
-```
-
-#### Add a role to a group
-
-```bash
-gcli group grant --group groupA --role admin
-```
+#### Simple authentication with user name
-#### Remove a role from a group
```bash
-gcli group revoke --group groupA --role admin
+gcli <normal command> --simple --login userName
Review Comment:
?
```suggestion
gcli <normal command> --auth simple --login userName
```
##########
clients/cli/src/main/java/org/apache/gravitino/cli/commands/AllMetalakeDetails.java:
##########
@@ -32,9 +32,12 @@ public class AllMetalakeDetails extends Command {
*
* @param url The URL of the Gravitino server.
* @param ignoreVersions If true don't check the client/server versions
match.
+ * @param authentication Authentication type i.e. "simple"
+ * @param userName User name for simple authentication.
*/
- public AllMetalakeDetails(String url, boolean ignoreVersions) {
- super(url, ignoreVersions);
+ public AllMetalakeDetails(
+ String url, boolean ignoreVersions, String authentication, String
userName) {
+ super(url, ignoreVersions, authentication, userName);
Review Comment:
```suggestion
String url, boolean ignoreVersions, String authType, String userName) {
super(url, ignoreVersions, authType, userName);
```
##########
docs/cli.md:
##########
@@ -522,51 +533,15 @@ gcli tag update --tag tagA --rename newTag
gcli tag update --tag tagA --comment "new comment"
```
-### Role commands
+### Authentication
-#### Display role details
+#### Simple authentication
```bash
-gcli role details --role admin
-```
-
-#### List all roles
-
-```bash
-gcli role list
-```
-
-#### Create a role
-
-```bash
-gcli role create --role admin
-```
-
-#### Delete a role
+gcli <normal command> --simple
Review Comment:
?
```suggestion
gcli <normal command> --auth simple
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
