You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2020/01/03 11:09:38 UTC

[GitHub] [incubator-hudi] pratyakshsharma opened a new pull request #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

pratyakshsharma opened a new pull request #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174
 
 
   1. Implemented command line options replacing positional arguments when launching spark applications from CLI commands. 
   2. Created AbstractCommandConfig class as the base class for all the CLI specific config objects. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] vinothchandar commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
vinothchandar commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174#issuecomment-611059024
 
 
   @pratyakshsharma sure! can you rebase and let us know if its ready for a final review and merge cycle? 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] codecov-io commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174#issuecomment-598671510
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1174?src=pr&el=h1) Report
   > Merging [#1174](https://codecov.io/gh/apache/incubator-hudi/pull/1174?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-hudi/commit/fb7fba365f89afaf97cc969ecc09f3bbfc4a5dbd&el=desc) will **decrease** coverage by `0.23%`.
   > The diff coverage is `6.14%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-hudi/pull/1174/graphs/tree.svg?width=650&height=150&src=pr&token=VTTXabwbs2)](https://codecov.io/gh/apache/incubator-hudi/pull/1174?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1174      +/-   ##
   ============================================
   - Coverage     67.50%   67.26%   -0.24%     
   - Complexity      230      231       +1     
   ============================================
     Files           337      340       +3     
     Lines         16354    16409      +55     
     Branches       1671     1675       +4     
   ============================================
   - Hits          11040    11038       -2     
   - Misses         4579     4636      +57     
     Partials        735      735              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-hudi/pull/1174?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [.../java/org/apache/hudi/utilities/HoodieCleaner.java](https://codecov.io/gh/apache/incubator-hudi/pull/1174/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0hvb2RpZUNsZWFuZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ache/hudi/utilities/HoodieCompactionAdminTool.java](https://codecov.io/gh/apache/incubator-hudi/pull/1174/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0hvb2RpZUNvbXBhY3Rpb25BZG1pblRvb2wuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ava/org/apache/hudi/utilities/HoodieCompactor.java](https://codecov.io/gh/apache/incubator-hudi/pull/1174/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0hvb2RpZUNvbXBhY3Rvci5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...java/org/apache/hudi/utilities/HoodieRollback.java](https://codecov.io/gh/apache/incubator-hudi/pull/1174/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0hvb2RpZVJvbGxiYWNrLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...rg/apache/hudi/utilities/HoodieSnapshotCopier.java](https://codecov.io/gh/apache/incubator-hudi/pull/1174/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0hvb2RpZVNuYXBzaG90Q29waWVyLmphdmE=) | `14.10% <0.00%> (ø)` | `3.00 <0.00> (ø)` | |
   | [...ities/exception/InvalidCommandConfigException.java](https://codecov.io/gh/apache/incubator-hudi/pull/1174/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2V4Y2VwdGlvbi9JbnZhbGlkQ29tbWFuZENvbmZpZ0V4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...e/hudi/utilities/config/AbstractCommandConfig.java](https://codecov.io/gh/apache/incubator-hudi/pull/1174/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2NvbmZpZy9BYnN0cmFjdENvbW1hbmRDb25maWcuamF2YQ==) | `5.35% <5.35%> (ø)` | `1.00 <1.00> (?)` | |
   | [...in/java/org/apache/hudi/utilities/UtilHelpers.java](https://codecov.io/gh/apache/incubator-hudi/pull/1174/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL1V0aWxIZWxwZXJzLmphdmE=) | `63.07% <33.33%> (+0.28%)` | `20.00 <1.00> (ø)` | |
   | [...org/apache/hudi/utilities/HDFSParquetImporter.java](https://codecov.io/gh/apache/incubator-hudi/pull/1174/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0hERlNQYXJxdWV0SW1wb3J0ZXIuamF2YQ==) | `69.00% <50.00%> (+1.38%)` | `11.00 <0.00> (ø)` | |
   | [...g/apache/hudi/utilities/HiveIncrementalPuller.java](https://codecov.io/gh/apache/incubator-hudi/pull/1174/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0hpdmVJbmNyZW1lbnRhbFB1bGxlci5qYXZh) | `13.81% <50.00%> (-0.84%)` | `3.00 <0.00> (ø)` | |
   | ... and [4 more](https://codecov.io/gh/apache/incubator-hudi/pull/1174/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1174?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1174?src=pr&el=footer). Last update [fb7fba3...f42ae5e](https://codecov.io/gh/apache/incubator-hudi/pull/1174?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] n3nash commented on issue #1174: [WIP] [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
n3nash commented on issue #1174: [WIP] [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174#issuecomment-573521876
 
 
   I'll review this by EOD tomorrow

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] pratyakshsharma commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174#issuecomment-609676490
 
 
   @vinothchandar @n3nash lets revive this? :) 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] vinothchandar commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
vinothchandar commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174#issuecomment-612703547
 
 
   @pratyakshsharma @n3nash this has been around for sometime.. does @hddong also have a PR for the same JIRA?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] vinothchandar commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
vinothchandar commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174#issuecomment-571201084
 
 
   @n3nash could you please review this?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] pratyakshsharma commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174#issuecomment-597638791
 
 
   @n3nash Did you get a chance to review it again? Was thinking of rebasing it along with addressing your comments. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] hddong commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
hddong commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174#issuecomment-612798425
 
 
   @vinothchandar : as @pratyakshsharma said, it's not same as this PR.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] pratyakshsharma commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174#issuecomment-612790506
 
 
   > @pratyakshsharma @n3nash this has been around for sometime.. does @hddong also have a PR for the same JIRA?
   
   @hddong had a PR to fix specifying sparkMaster and sparkMemory at one place. It involved some code refactoring but it did not fix positional arguments. Here it is - https://github.com/apache/incubator-hudi/pull/1452
   
   Yesterday I rebased the branch with master and integration tests are breaking. I am working on fixing them. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] n3nash edited a comment on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
n3nash edited a comment on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174#issuecomment-582242214
 
 
   Just checked this PR and for some reason my comments weren't submitted last time I added a review, adding them again, expect them in a couple of hours

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] n3nash commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
n3nash commented on issue #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174#issuecomment-582242214
 
 
   For some reason my comments weren't submitted last time I added a review, adding them again, expect them in a couple of hours

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] pratyakshsharma commented on a change in pull request #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on a change in pull request #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174#discussion_r407205799
 
 

 ##########
 File path: hudi-cli/src/main/java/org/apache/hudi/cli/commands/CleansCommand.java
 ##########
 @@ -136,9 +139,17 @@ public String runClean(@CliOption(key = "sparkMemory", unspecifiedDefaultValue =
         Utils.getDefaultPropertiesFile(JavaConverters.mapAsScalaMapConverter(System.getenv()).asScala());
     SparkLauncher sparkLauncher = SparkUtil.initLauncher(sparkPropertiesPath);
 
-    String cmd = SparkMain.SparkCommand.CLEAN.toString();
-    sparkLauncher.addAppArgs(cmd, metaClient.getBasePath(), master, propsFilePath, sparkMemory);
-    UtilHelpers.validateAndAddProperties(configs, sparkLauncher);
+    HoodieCleaner.Config config = new HoodieCleaner.Config();
+    config.configs = Arrays.asList(configs);
+    if (!Strings.isNullOrEmpty(propsFilePath)) {
+      config.propsFilePath = propsFilePath;
+    }
+    config.basePath = metaClient.getBasePath();
+    config.sparkMaster = master;
+    config.sparkMemory = sparkMemory;
+    String[] commandConfig = config.getCommandConfigsAsStringArray(SparkMain.SparkCommand.CLEAN.toString());
+    sparkLauncher.addAppArgs(commandConfig);
 
 Review comment:
   The purpose of this PR is to remove this passing of positional arguments and instead have a config object. Since passing and managing lot of arguments can get difficult at times. @hddong 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hudi] vinothchandar commented on pull request #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
vinothchandar commented on pull request #1174:
URL: https://github.com/apache/hudi/pull/1174#issuecomment-914630077


   Closing due to inactivty


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-hudi] hddong commented on a change in pull request #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands
URL: https://github.com/apache/incubator-hudi/pull/1174#discussion_r399733999
 
 

 ##########
 File path: hudi-cli/src/main/java/org/apache/hudi/cli/commands/CleansCommand.java
 ##########
 @@ -136,9 +139,17 @@ public String runClean(@CliOption(key = "sparkMemory", unspecifiedDefaultValue =
         Utils.getDefaultPropertiesFile(JavaConverters.mapAsScalaMapConverter(System.getenv()).asScala());
     SparkLauncher sparkLauncher = SparkUtil.initLauncher(sparkPropertiesPath);
 
-    String cmd = SparkMain.SparkCommand.CLEAN.toString();
-    sparkLauncher.addAppArgs(cmd, metaClient.getBasePath(), master, propsFilePath, sparkMemory);
-    UtilHelpers.validateAndAddProperties(configs, sparkLauncher);
+    HoodieCleaner.Config config = new HoodieCleaner.Config();
+    config.configs = Arrays.asList(configs);
+    if (!Strings.isNullOrEmpty(propsFilePath)) {
+      config.propsFilePath = propsFilePath;
+    }
+    config.basePath = metaClient.getBasePath();
+    config.sparkMaster = master;
+    config.sparkMemory = sparkMemory;
+    String[] commandConfig = config.getCommandConfigsAsStringArray(SparkMain.SparkCommand.CLEAN.toString());
+    sparkLauncher.addAppArgs(commandConfig);
 
 Review comment:
   Need we construct `Config` class here? How about passing raw parameters and construct config in `SparkMain`,  like `compact` function does.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hudi] vinothchandar closed pull request #1174: [HUDI-96]: Implemented command line options instead of positional arguments for CLI commands

Posted by GitBox <gi...@apache.org>.
vinothchandar closed pull request #1174:
URL: https://github.com/apache/hudi/pull/1174


   


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org