mengw15 commented on code in PR #7602:
URL: https://github.com/apache/texera/pull/7602#discussion_r3781297619
##########
notebook-migration-service/src/main/scala/org/apache/texera/service/resource/NotebookMigrationResource.scala:
##########
@@ -75,13 +75,10 @@ object NotebookMigrationResource extends LazyLogging {
private val jupyterUrl = StorageConfig.jupyterURL
private val jupyterToken = StorageConfig.jupyterToken
- // The token is passed as a URL param so the browser iframe can authenticate
when loading the notebook.
- // jupyterIframeURL is process-global state. This is safe ONLY because each
user runs their own pod
- // (own notebook-migration-service JVM + own Jupyter) in the k8s deployment,
so this singleton is
- // effectively per-user. Do NOT deploy this service as a shared multi-user
instance without adding
- // per-user keying here, or one user's upload would overwrite another's
iframe URL.
- @volatile private var jupyterIframeURL =
- s"$jupyterUrl/notebooks/work/notebook.ipynb?token=$jupyterToken"
+
+ // Default notebook name used when a request does not specify one, so a
param-less
+ // getJupyterIframeURL call reproduces the URL from before this service
became stateless.
+ private val defaultNotebookName = "notebook.ipynb"
Review Comment:
Following up: #7390 notes the shared state "blocks running the service as a
single global instance", and it closes with this PR — so with the warning
comment going too, I'm not sure what's left tracking the
jupyterUrl/jupyterToken half. Maybe a follow-up issue like #7636?
--
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]
