danny0405 commented on code in PR #19970:
URL: https://github.com/apache/hudi/pull/19970#discussion_r4129132716
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/SparkMain.java:
##########
@@ -139,8 +170,96 @@ public static void addAppArgs(SparkLauncher sparkLauncher,
SparkMain.SparkComman
sparkLauncher.addAppArgs(args);
}
- public static void main(String[] args) {
- ValidationUtils.checkArgument(args.length >= 4);
+ /**
+ * Adds the command invocation as fully named options: {@code --command X
--master Y --memory Z}
+ * plus {@code -Dkey=value} pairs for every command argument. This makes the
launcher command
+ * self-describing and removes the risk of mis-aligned positional arguments
at call sites.
+ *
+ * @param namedArgs flat name/value pairs; {@code null} values are skipped,
preserving the
+ * legacy semantics of absent trailing arguments (e.g.
propsFilePath)
+ */
+ public static void addNamedAppArgs(SparkLauncher sparkLauncher,
SparkMain.SparkCommand cmd, String master, String memory, String... namedArgs) {
+ ValidationUtils.checkArgument(namedArgs.length % 2 == 0, "namedArgs must
be flat name/value pairs");
+ sparkLauncher.addAppArgs("--command", cmd.toString());
+ sparkLauncher.addAppArgs("--master", master);
+ sparkLauncher.addAppArgs("--memory", memory);
+ for (int i = 0; i < namedArgs.length; i += 2) {
+ if (namedArgs[i + 1] != null) {
+ sparkLauncher.addAppArgs("-D" + namedArgs[i] + "=" + namedArgs[i + 1]);
+ }
+ }
+ }
+
+ private static boolean isNamedInvocation(String[] argv) {
+ return argv.length > 0 && argv[0].startsWith("-");
+ }
+
+ /**
+ * Translates {@code --command X --master Y --memory Z -Dkey=value...} into
the positional
+ * form {@code [X, Y, Z, arg1, arg2, ...]} expected by the legacy invocation
path. The named
+ * -D arguments are mapped onto the command's positional argument names;
unknown -D keys are
+ * passed through as trailing configs, matching {@link
SparkCommand#makeConfigs(String[])}.
+ */
+ private static String[] namedArgsToPositional(String[] argv) {
+ NamedArgs namedArgs = new NamedArgs();
+ JCommander commander = JCommander.newBuilder()
+ .addObject(namedArgs)
+ .build();
+ commander.parse(argv);
+
+ SparkCommand command = SparkCommand.valueOf(namedArgs.command);
+ Map<String, String> params = namedArgs.params;
+ List<String> positionalArgs = new ArrayList<>(command.paramNames.length);
+ for (String paramName : command.paramNames) {
+ String value = params.get(paramName);
+ // absent named args map to empty strings to keep the positional slots
aligned; trailing
+ // empty slots are handled by getPropsFilePath/makeConfigs exactly like
the legacy form
+ positionalArgs.add(value != null ? value.trim() : "");
Review Comment:
[P2] Preserve whitespace in command argument values
Trimming every value changes the meaning of paths and partition values that
the positional form preserved. For example, passing `oldPartition="region=us "`
through the new launcher/translator produces `region=us` (reproduced using the
PR translation code and JCommander 1.78). If both partitions exist,
`renamePartition()` reads, rewrites, and deletes the partition without the
trailing space instead of the requested partition. This also changes savepoint
comments and other string arguments. Please preserve string values end-to-end,
including any normalization performed by JCommander, and add a round-trip test
for leading/trailing whitespace.
--
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]
