Conversation
|
I will revert the changes to the dashboard lock file and apply them in a separate PR - it probably makes things clearer. |
There was a problem hiding this comment.
Pull request overview
This PR is a WIP pass to standardize conda environment naming (and related container naming) across the dashboard (GUI) and ML training components.
Changes:
- Renames conda environments to
synapse-guiandsynapse-ml, updating dashboard entrypoint/docs accordingly. - Updates container image naming in
publish_container.pytosynapse-gui/synapse-ml. - Adjusts dependency constraints (e.g., relaxes pins in the dashboard env; bumps
lume-modelminimum in ML env).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| publish_container.py | Updates published Docker image names to synapse-gui / synapse-ml. |
| ml/environment.yml | Renames ML conda env to synapse-ml and bumps lume-model constraint. |
| dashboard/environment.yml | Renames GUI conda env to synapse-gui and relaxes some dependency pins. |
| dashboard/entrypoint.sh | Updates conda activation to the renamed GUI environment. |
| dashboard/README.md | Updates lockfile/env naming and Docker instructions to the new GUI names. |
| dashboard.Dockerfile | Switches to the renamed lockfile and installs the renamed GUI environment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A counterargument to this, which I think is partially pointed out by GitHub Copilot's review as well, is that the lock file and the base file would be inconsistent if one merges the PR without the lock file changes and then checks out that resulting commit in |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
dashboard/environment-lock.yml:12
- This lock file appears to have been edited to rename the source/lockfile references, but the package solution +
metadata.content_hashshould be regenerated wheneverenvironment.ymlchanges (env name/version constraints). Otherwise the lock may not actually correspond to the currentdashboard/environment.yml, which is confusing and can lead to installing unexpected versions.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - plotly<6 | ||
| - plotly | ||
| - pymongo | ||
| - python<3.13 |
There was a problem hiding this comment.
The version limit <3.13 here is necessary due to the following problem diagnosed by Claude Opus 4.5:
Python 3.12+ changed
asyncio.get_event_loop()behavior - it now raisesRuntimeErrorwhen called without a running event loop in the main thread (previously it would silently create one). Thewslinklibrary (a dependency oftrame) still uses this deprecated pattern in itsAbstractWebApp.__init__.
With later Python versions, we currently encounter this error:
Traceback (most recent call last):
File "/home/edoardo/src/synapse/dashboard/app.py", line 450, in <module>
server.start()
~~~~~~~~~~~~^^
File "/home/edoardo/miniconda3/envs/synapse-gui/lib/python3.14/site-packages/trame_server/core.py", line 716, in start
task = CoreServer.server_start(
options,
...<5 lines>...
},
)
File "/home/edoardo/miniconda3/envs/synapse-gui/lib/python3.14/site-packages/trame_server/protocol.py", line 51, in server_start
return server.start_webserver(
~~~~~~~~~~~~~~~~~~~~~~^
options=options,
^^^^^^^^^^^^^^^^
...<4 lines>...
**kwargs,
^^^^^^^^^
)
^
File "/home/edoardo/miniconda3/envs/synapse-gui/lib/python3.14/site-packages/wslink/server.py", line 258, in start_webserver
ws_server = create_webserver(server_config, backend=backend)
File "/home/edoardo/miniconda3/envs/synapse-gui/lib/python3.14/site-packages/wslink/server.py", line 168, in create_webserver
return backends.create_webserver(server_config, backend=backend)
~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/edoardo/miniconda3/envs/synapse-gui/lib/python3.14/site-packages/wslink/backends/__init__.py", line 5, in create_webserver
return create_webserver(server_config)
File "/home/edoardo/miniconda3/envs/synapse-gui/lib/python3.14/site-packages/wslink/backends/aiohttp/__init__.py", line 217, in create_webserver
return WebAppServer(server_config)
File "/home/edoardo/miniconda3/envs/synapse-gui/lib/python3.14/site-packages/wslink/backends/aiohttp/__init__.py", line 74, in __init__
AbstractWebApp.__init__(self, server_config)
~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^
File "/home/edoardo/miniconda3/envs/synapse-gui/lib/python3.14/site-packages/wslink/protocol.py", line 33, in __init__
self._completion = asyncio.get_event_loop().create_future()
~~~~~~~~~~~~~~~~~~~~~~^^
File "/home/edoardo/miniconda3/envs/synapse-gui/lib/python3.14/asyncio/events.py", line 715, in get_event_loop
raise RuntimeError('There is no current event loop in thread %r.'
% threading.current_thread().name)
RuntimeError: There is no current event loop in thread 'MainThread'.
Overview
I think the various conda environments (dashboard, ML training, documentation in #341) should be cleand up to ensure consistency, in terms of:
This is what I brainstormed with Claude (inquiring about common practices for conda environments, etc.):
To do
python,botorch, andlume-model: