paul8263 commented on code in PR #6489:
URL: https://github.com/apache/hudi/pull/6489#discussion_r969175549


##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/UpgradeOrDowngradeCommand.java:
##########
@@ -25,25 +25,24 @@
 import org.apache.hudi.common.table.HoodieTableMetaClient;
 import org.apache.hudi.common.table.HoodieTableVersion;
 import org.apache.hudi.common.util.StringUtils;
-
 import org.apache.spark.launcher.SparkLauncher;
-import org.springframework.shell.core.CommandMarker;
-import org.springframework.shell.core.annotation.CliCommand;
-import org.springframework.shell.core.annotation.CliOption;
-import org.springframework.stereotype.Component;
+import org.springframework.shell.standard.ShellComponent;
+import org.springframework.shell.standard.ShellMethod;
+import org.springframework.shell.standard.ShellOption;
 
 /**
  * CLI command to assist in upgrading/downgrading Hoodie table to a different 
version.
  */
-@Component
-public class UpgradeOrDowngradeCommand implements CommandMarker {
+@ShellComponent
+public class UpgradeOrDowngradeCommand {
 
-  @CliCommand(value = "upgrade table", help = "Upgrades a table")
+  @ShellMethod(key = "upgrade table", value = "Upgrades a table")
   public String upgradeHoodieTable(
-      @CliOption(key = {"toVersion"}, help = "To version of Hoodie table to be 
upgraded/downgraded to", unspecifiedDefaultValue = "") final String toVersion,
-      @CliOption(key = {"sparkProperties"}, help = "Spark Properties File 
Path") final String sparkPropertiesPath,
-      @CliOption(key = "sparkMaster", unspecifiedDefaultValue = "", help = 
"Spark Master") String master,
-      @CliOption(key = "sparkMemory", unspecifiedDefaultValue = "4G",
+      @ShellOption(value = {"--toVersion"}, help = "To version of Hoodie table 
to be upgraded/downgraded to", defaultValue = "") final String toVersion,
+      @ShellOption(value = {"--sparkProperties"}, help = "Spark Properties 
File Path",
+          defaultValue = ShellOption.NULL) final String sparkPropertiesPath,

Review Comment:
   "--sparkProperties" does not have to be mandatory. As we can see in 
`hudi-cli/src/main/java/org/apache/hudi/cli/utils/SparkUtil.java::initLauncher`
   `    if (!StringUtils.isNullOrEmpty(propertiesFile)) {
         sparkLauncher.setPropertiesFile(propertiesFile);
       }`
   
   If propertiesFile was null or empty, SparkLauncher would not set the 
propertiesFile any more.
   
   I agree with you that we had better set the default value of 
`--sparkProperties` as an empty string. 



-- 
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]

Reply via email to