Skip to content

[KYUUBI #7415] Add a configuration to include additional Spark keys as path validation targets#7416

Closed
LamiumAmplexicaule wants to merge 4 commits into
apache:masterfrom
LamiumAmplexicaule:add-spark-keys-as-check-target
Closed

[KYUUBI #7415] Add a configuration to include additional Spark keys as path validation targets#7416
LamiumAmplexicaule wants to merge 4 commits into
apache:masterfrom
LamiumAmplexicaule:add-spark-keys-as-check-target

Conversation

@LamiumAmplexicaule

Copy link
Copy Markdown
Contributor

Why are the changes needed?

To add Spark configuration keys for path validation without rebuilding.
Close: #7415

How was this patch tested?

Unit tests.

$ ./build/mvn clean install -Dtest=none -DwildcardSuites=org.apache.kyuubi.engine.KyuubiApplicationManagerSuite

Was this patch authored or co-authored using generative AI tooling?

No.

Comment thread kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala Outdated
@pan3793

pan3793 commented Apr 24, 2026

Copy link
Copy Markdown
Member

is this config allow the user to override with the session conf? or is it a server-side static config? - unfortunately, our codebase does not use a canonical name to distinguish serverConf, sessionConf, opConf, etc., and this has been causing confusion and bugs

@LamiumAmplexicaule LamiumAmplexicaule force-pushed the add-spark-keys-as-check-target branch from 23380f7 to 61fc167 Compare April 24, 2026 11:58
@LamiumAmplexicaule

LamiumAmplexicaule commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for your question.

is this config allow the user to override with the session conf? or is it a server-side static config?

Since it must not be overwritten, I think it should be a static server-side config.
Also, the conf used in this PR is the KyuubiSessionManager config, so I consider it a server-side static config.

override def checkSessionAccessPathURIs(): Unit = {
KyuubiApplicationManager.checkApplicationAccessPaths(
sessionConf.get(ENGINE_TYPE),
sessionConf.getAll,
sessionManager.getConf)
}

def checkApplicationAccessPaths(
applicationType: String,
appConf: Map[String, String],
kyuubiConf: KyuubiConf): Unit = {
applicationType.toUpperCase(Locale.ROOT) match {
case appType if appType.contains("SPARK") => checkSparkAccessPaths(appConf, kyuubiConf)
case appType if appType.startsWith("FLINK") => // TODO: check flink app access local paths
case _ =>
}
}

.createWithDefault(Set.empty)

val SESSION_SPARK_FILE_CONFIG_LIST: ConfigEntry[Set[String]] =
buildConf("kyuubi.session.spark.file.config.list")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's put it under kyuubi.server. namespace as it's a server-side static config.

Suggested change
buildConf("kyuubi.session.spark.file.config.list")
buildConf("kyuubi.server.spark.file.config.list")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed the config to kyuubi.server.spark.file.config.list in 2f8450c.

@pan3793 pan3793 added this to the v1.12.0 milestone Apr 24, 2026
@pan3793

pan3793 commented Apr 24, 2026

Copy link
Copy Markdown
Member

change the config namespace, otherwise lgtm

@aajisaka aajisaka closed this in c6b01db Apr 28, 2026
@aajisaka

Copy link
Copy Markdown
Member

Merged the PR into master. Thank you @LamiumAmplexicaule @pan3793 !

@LamiumAmplexicaule LamiumAmplexicaule deleted the add-spark-keys-as-check-target branch April 28, 2026 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Enable adding Spark parameters that Kyuubi validate without rebuilding, similar to Livy

3 participants