lasdf1234 commented on code in PR #13134:
URL: https://github.com/apache/gravitino/pull/13134#discussion_r4024026841
##########
core/src/main/java/org/apache/gravitino/job/JobManager.java:
##########
@@ -841,35 +844,41 @@ public static JobTemplate createRuntimeJobTemplate(
String comment = jobTemplateEntity.comment();
JobTemplateEntity.TemplateContent content =
jobTemplateEntity.templateContent();
- String executable =
- fetchFileFromUri(
- replacePlaceholder(content.executable(), jobConf), stagingDir,
TIMEOUT_IN_MS);
+ String executableUri = replacePlaceholder(content.executable(), jobConf);
+ rejectEmbeddedUnresolvedPlaceholder(executableUri, "executable");
+ String executable = fetchFileFromUri(executableUri, stagingDir,
TIMEOUT_IN_MS);
List<String> args =
Review Comment:
Thanks for your review. IcebergJobUtils.parseArguments now keeps explicit
empty values, so JobManager’s “preserve empty jobConf values” contract is
observable in the built-in Iceberg jobs. Required args still treat blank as
missing via trimToNull.
##########
core/src/main/java/org/apache/gravitino/job/JobManager.java:
##########
@@ -886,35 +895,39 @@ public static JobTemplate createRuntimeJobTemplate(
// For Spark job template
if (content.jobType() == JobTemplate.JobType.SPARK) {
String className = replacePlaceholder(content.className(), jobConf);
+ rejectEmbeddedUnresolvedPlaceholder(className, "className");
List<String> jars =
content.jars().stream()
.map(
- jar ->
- fetchFileFromUri(replacePlaceholder(jar, jobConf),
stagingDir, TIMEOUT_IN_MS))
+ jar -> {
+ String resolved = replacePlaceholder(jar, jobConf);
+ rejectEmbeddedUnresolvedPlaceholder(resolved, "jar");
+ return fetchFileFromUri(resolved, stagingDir,
TIMEOUT_IN_MS);
+ })
.collect(Collectors.toList());
List<String> files =
content.files().stream()
.map(
- file ->
- fetchFileFromUri(
- replacePlaceholder(file, jobConf), stagingDir,
TIMEOUT_IN_MS))
+ file -> {
+ String resolved = replacePlaceholder(file, jobConf);
+ rejectEmbeddedUnresolvedPlaceholder(resolved, "file");
+ return fetchFileFromUri(resolved, stagingDir,
TIMEOUT_IN_MS);
+ })
.collect(Collectors.toList());
List<String> archives =
content.archives().stream()
.map(
- archive ->
- fetchFileFromUri(
- replacePlaceholder(archive, jobConf), stagingDir,
TIMEOUT_IN_MS))
+ archive -> {
+ String resolved = replacePlaceholder(archive, jobConf);
+ rejectEmbeddedUnresolvedPlaceholder(resolved, "archive");
+ return fetchFileFromUri(resolved, stagingDir,
TIMEOUT_IN_MS);
+ })
.collect(Collectors.toList());
Map<String, String> configs =
- content.configs().entrySet().stream()
- .collect(
- Collectors.toMap(
- entry -> replacePlaceholder(entry.getKey(), jobConf),
- entry -> replacePlaceholder(entry.getValue(), jobConf)));
+ omitUnresolvedTemplateMap(content.configs(), jobConf, "configs");
Review Comment:
Thanks for the review. Unresolved placeholders in config/environment keys
(e.g. spark.sql.catalog.{{catalog_name}}) are now dropped instead of rejected,
so omitting catalog_name no longer surfaces as a JobManager-level
embedded-placeholder error.
##########
maintenance/jobs/src/main/java/org/apache/gravitino/maintenance/jobs/iceberg/IcebergRewriteDataFilesJob.java:
##########
@@ -407,19 +426,19 @@ static Map<String, String> parseOptionsJson(String
optionsJson) {
*/
private static List<String> buildArguments() {
return Arrays.asList(
- "--catalog",
+ "--" + IcebergJobUtils.OPTION_CATALOG,
"{{catalog_name}}",
- "--table",
+ "--" + IcebergJobUtils.OPTION_TABLE,
"{{table_identifier}}",
- "--strategy",
+ "--" + OPTION_STRATEGY,
"{{strategy}}",
- "--sort-order",
+ "--" + OPTION_SORT_ORDER,
"{{sort_order}}",
- "--where",
+ "--" + OPTION_WHERE,
Review Comment:
Thank you very much for your review. This issue has been fixed. OPTION_WHERE
= "where-clause" is now consistent with the {{where_clause}} specification.
Additionally, --catalog / --table, etc. have also been modified to align with
the placeholders, and the "TestBuiltInIcebergJobFlagPlaceholderAlignment" has
been added.
--
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]
