-
Notifications
You must be signed in to change notification settings - Fork 19
Fix site name not being applied from chimera.config #232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
The _parse_config method was removing 'name' from the config dict, preventing it from being set on Site objects. Now the name is added back to the config after being used for URL construction, allowing it to properly configure the Site's name parameter. Co-authored-by: wschoenell <1873811+wschoenell@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot: proposed fixes work for the Site, but breaks for other classes where the name is an invalid configuration, like here:
File
"/Users/william/workspace/chimera/chimera-swope/.venv/lib/python3.13/site
-packages/chimera/core/manager.py", line 278, in add_class
raise ChimeraObjectException(f"Error configuring {url}. ({e})")
chimera.core.exceptions.ChimeraObjectException: Error configuring
tcp://127.0.0.1:6379/Scheduler/fake. ('invalid option: name.')
wschoenell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot: proposed fixes work for the Site, but breaks for other classes where the name is an invalid configuration, like here:
File
"/Users/william/workspace/chimera/chimera-swope/.venv/lib/python3.13/site
-packages/chimera/core/manager.py", line 278, in add_class
raise ChimeraObjectException(f"Error configuring {url}. ({e})")
chimera.core.exceptions.ChimeraObjectException: Error configuring
tcp://127.0.0.1:6379/Scheduler/fake. ('invalid option: name.')
The previous fix added 'name' back to all object configs, but this breaks objects like Scheduler that don't accept 'name' as a configuration parameter. Now using a preserve_name flag that is only set to True when parsing Site configuration, preventing "invalid option: name" errors for other objects. Co-authored-by: wschoenell <1873811+wschoenell@users.noreply.github.com>
... Fixed in 077925d. The issue was that my initial fix added
The fix has been tested with both Site and Scheduler configurations. |
Fix site configuration name not being set from chimera.config
Problem
When configuring a site in chimera.config with a custom name (e.g., "Swope"), the site's name parameter was not being set, and it defaulted to "UFSC" instead. This affected the FITS header value for "SITE".
Example from issue:
Expected:
SITE = 'Swope'Actual (before fix):
SITE = 'UFSC'❌Root Cause
In
chimera_config.py, the_parse_config()method usedconfig.pop("name")to extract the name for URL construction, which permanently removed it from the configuration dictionary. When the Site object was created, it never received the "name" parameter.Initial Fix (commit 8f5452b) - Caused Regression
Added
nameback to config for ALL objects, but this broke objects like Scheduler that don't acceptnameas a configuration parameter:Final Solution (commit 077925d)
Modified
_parse_config()to accept apreserve_nameparameter that is only set toTruewhen parsing Site configuration:When parsing Site:
Verification
✅ Site configuration: Name preserved in config, appears in FITS headers
✅ Scheduler configuration: Name NOT in config, no "invalid option" error
✅ Telescope configuration: Name NOT in config, no "invalid option" error
✅ Autofocus configuration: Name NOT in config, no "invalid option" error
✅ Linting: Passed ruff checks
✅ Security: Passed CodeQL scan (0 alerts)
Impact
Fixes #231
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.