jerryshao commented on code in PR #9960:
URL: https://github.com/apache/gravitino/pull/9960#discussion_r2964460311
##########
api/src/main/java/org/apache/gravitino/job/JobTemplate.java:
##########
@@ -73,7 +73,25 @@ public enum JobType {
/** The executable path for the job template. */
protected final String executable;
- /** The list of arguments for the job template. */
+ /**
+ * The list of arguments for the job template.
+ *
+ * <p>Arguments can be marked as optional by prefixing them with {@code ?}.
An optional argument
+ * is filtered out at job execution time if its effective value is
considered empty. A value is
+ * empty when it is {@code null}, an empty string, whitespace-only, or an
unreplaced placeholder
+ * (i.e., the key was absent from the job configuration).
+ *
+ * <p>Two consecutive optional arguments can form a flag-value pair. When
the flag (the first
Review Comment:
> IMO, if we simplify the rule to only drop the empty `?` placeholder, we'll
hit the "dangling flag" issue, causing CLI execution failures.
>
> Many CLIs require space-separated arguments. If a user writes `["--conf",
"?{{spark_conf}}", "my_app.jar"]` and `spark_conf` is empty:
>
> * Without pairing (Simplified): It becomes `["--conf", "my_app.jar"]`.
Spark mistakenly uses `"my_app.jar"` as the config value, and the job crashes
because the main jar is missing.
> * With pairing (Current): Both the flag and empty value drop safely,
resulting in `["my_app.jar"]`.
>
> We can't rely on the `=` syntax (e.g., `"?--conf={{spark_conf}}"`) because
many tools don't support it for short flags.
>
> To reduce overhead, I can add this explicit example to the Javadoc/docs.
What do you think?
Yes, this is a potential issue. But, can we leave this to the user? It is
the user's responsibility to correctly use the `?`. My feeling is that even if
we have a sophisticated check mechanism, there still could have corner cases.
So I'm inclined to leave this to the users to guarantee the right behavior.
What do you think?
--
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]
