zyratlo commented on code in PR #5258:
URL: https://github.com/apache/texera/pull/5258#discussion_r3461824726
##########
common/config/src/main/scala/org/apache/texera/common/config/StorageConfig.scala:
##########
@@ -133,4 +133,8 @@ object StorageConfig {
val ENV_S3_REGION = "STORAGE_S3_REGION"
val ENV_S3_AUTH_USERNAME = "STORAGE_S3_AUTH_USERNAME"
val ENV_S3_AUTH_PASSWORD = "STORAGE_S3_AUTH_PASSWORD"
+
+ // Jupyter
+ val jupyterURL: String = conf.getString("storage.jupyter.url")
+ val jupyterToken: String = conf.getString("storage.jupyter.token")
Review Comment:
I'm going to leave this as-is. StorageConfig's ENV_* constants aren't
actually referenced anywhere — the env-var registry that's consumed at runtime
is common/config/.../EnvironmentalVariable.scala (used by
ComputingUnitManagingResource to pass settings into spawned pods). So adding
ENV_JUPYTER_* constants here would only grow an unused, duplicative block
rather than improve the config surface.
If/when the Jupyter settings need to propagate to spawned pods, the right
place to add them is EnvironmentalVariable — but that wiring doesn't exist yet,
so I'd add the constant when it's actually needed rather than speculatively.
The env var names themselves are already documented where they're consumed:
storage.conf (STORAGE_JUPYTER_URL, JUPYTER_TOKEN).
--
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]
