echonesis commented on code in PR #9960:
URL: https://github.com/apache/gravitino/pull/9960#discussion_r2935438559
##########
core/src/main/java/org/apache/gravitino/job/JobManager.java:
##########
@@ -787,6 +793,174 @@ static String replacePlaceholder(String inputString,
Map<String, String> replace
return result.toString();
}
+ /**
+ * Build arguments list with optional argument support.
+ *
+ * <p>Arguments prefixed with {@code ?} are <em>optional</em>. An optional
argument is omitted
+ * from the final list when its resolved value is considered empty (see
{@link
+ * #isEmptyValue(String)}). "Empty" means the value is {@code null}, blank,
or an unreplaced
+ * placeholder (e.g. {@code {{key}}} where {@code key} is absent from {@code
jobConf}).
+ *
+ * <p><b>Flag–value pairs</b>: a {@code ?}-prefixed <em>literal flag</em>
and the immediately
+ * following value argument form a pair when the value contains a
placeholder. The entire pair is
+ * skipped when the value resolves to empty, <em>regardless of whether the
value argument itself
+ * carries a {@code ?} prefix</em>. When the value is non-empty both the
flag and the value are
+ * included.
+ *
+ * <p><b>Independent optional placeholders</b>: two consecutive {@code
?{{...}}} arguments (both
+ * are placeholders, neither is a literal flag) are evaluated
<em>independently</em>. The absence
+ * of one does not suppress the other.
+ *
+ * <p><b>Standalone optional literals</b>: a {@code ?}-prefixed literal flag
that is the last
+ * element of the list, or whose next neighbour is also a literal flag (not
a placeholder), is
+ * always included. There is no placeholder value to evaluate, so the {@code
?} has no effect. To
+ * make such a flag conditionally suppressible, pair it with a sentinel
optional placeholder and
+ * omit the sentinel key from {@code jobConf}:
+ *
+ * <pre>
+ * ["?--dry-run", "?{{dry_run_sentinel}}"] // omit dry_run_sentinel to
suppress both
+ * </pre>
+ *
+ * <p><b>Pattern reference</b>:
+ *
+ * <pre>
+ * Template pattern | jobConf | Result
+ *
-----------------------------------------+-------------------+---------------------------
+ * ["?--strategy", "?{{strategy}}"] | {strategy:binpack}|
["--strategy", "binpack"]
+ * ["?--strategy", "?{{strategy}}"] | {} | []
+ * ["?--strategy", "{{strategy}}"] | {strategy:binpack}|
["--strategy", "binpack"]
+ * ["?--strategy", "{{strategy}}"] | {} | []
(pair still skipped)
+ * ["?{{opt1}}", "?{{opt2}}"] | {opt1:v1} | ["v1"]
(independent)
+ * ["?--dry-run"] | {} |
["--dry-run"] (standalone)
Review Comment:
`?` means the argument is optional.
For a pattern like `?--strategy` followed by `{{strategy}}`, we treat them
as one optional flag-value pair:
- if strategy is absent, both are skipped
- if it is present, both are kept.
For a standalone literal flag like `?--dry-run`, `?` has no practical effect
in the current implementation, because there is no following placeholder value
to evaluate.
--
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]
