jerryshao commented on code in PR #13982:
URL: https://github.com/apache/gravitino/pull/13982#discussion_r4374981256
##########
maintenance/jobs/src/main/java/org/apache/gravitino/maintenance/jobs/iceberg/IcebergJobUtils.java:
##########
@@ -173,16 +202,69 @@ public static Map<String, String> parseArguments(
* @throws IllegalArgumentException if JSON parsing fails
*/
public static Map<String, String> parseCustomSparkConfigs(String
sparkConfJson) {
- return IcebergSparkConfigUtils.parseFlatJsonMap(sparkConfJson,
"spark-conf");
+ return filterCustomSparkConfigs(
+ IcebergSparkConfigUtils.parseFlatJsonMap(sparkConfJson, "spark-conf"));
+ }
+
+ /**
+ * Filter the custom Spark configurations of a job, which the driver applies
when it creates the
+ * SparkSession, against the configurations spark-submit has already set for
the driver.
+ *
+ * <p>{@code spark.master} and {@code spark.submit.deployMode} are dropped
if spark-submit has set
+ * them, as the job would otherwise run with another master than it was
submitted to. The {@code
+ * spark.kubernetes.*} configurations are dropped, as whoever runs a job
sets them here, while the
+ * driver creates the executor pods from them. A blank value leaves a
configuration unset.
+ *
+ * @param configs the custom Spark configurations
+ * @return the custom Spark configurations to apply
+ */
+ public static Map<String, String> filterCustomSparkConfigs(Map<String,
String> configs) {
+ // new SparkConf() reads the spark.* system properties on purpose:
spark-submit sets the
+ // configurations it submitted the job with as system properties before it
calls main().
+ return filterCustomSparkConfigs(configs, new SparkConf());
+ }
+
+ @VisibleForTesting
+ static Map<String, String> filterCustomSparkConfigs(
+ Map<String, String> configs, SparkConf submittedConf) {
+ Map<String, String> filtered = new LinkedHashMap<>();
+ configs.forEach(
+ (key, value) -> {
+ // A blank value leaves a configuration unset, as the k8s job
executor does.
+ if (value == null || value.trim().isEmpty()) {
Review Comment:
Switched to StringUtils.isBlank in d2804960c2, so the driver and the k8s
executor use the same blank check. Added a U+2003 (em space) value to
testFilterCustomSparkConfigsDropsKubernetesAndBlankConfigs.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
