Added settings to enable sending new structures and updates to instru…#51
Merged
jnation3406 merged 5 commits intomainfrom Jul 28, 2025
Merged
Added settings to enable sending new structures and updates to instru…#51jnation3406 merged 5 commits intomainfrom
jnation3406 merged 5 commits intomainfrom
Conversation
…ment capabilities to HEROIC
cmccully
approved these changes
Jun 24, 2025
cmccully
left a comment
There was a problem hiding this comment.
In general this looks fine to me. I'd like another review from someone who is more familiar with configdb though.
There was a problem hiding this comment.
Pull Request Overview
This PR integrates HEROIC reporting for instrument capabilities by adding settings, signal handlers, helper functions, and hooks in serializers/admin to trigger updates, along with tests to verify the behavior.
- Introduces HEROIC API environment settings and AppConfig conditional loading
- Implements
configdb.hardware.heroicmodule with helper functions andsend_to_heroic - Updates serializers and admin classes to call
update_heroic_instrument_capabilitieson relevant CRUD operations - Adds comprehensive API tests for HEROIC update triggers
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| configdb/settings.py | Added HEROIC API URL, token, and observatory env settings |
| configdb/hardware/tests.py | New TestHeroicUpdates class with mocked send_to_heroic tests |
| configdb/hardware/signals/handlers.py | Signal handlers to create/update HEROIC entities |
| configdb/hardware/serializers.py | Serializer update methods enhanced to trigger HEROIC updates |
| configdb/hardware/heroic.py | New HELPER functions for formatting and sending to HEROIC API |
| configdb/hardware/apps.py | can_submit_to_heroic and conditional signal registration |
| configdb/hardware/admin.py | Admin save_related/save_model overrides to trigger updates |
Comments suppressed due to low confidence (2)
configdb/hardware/tests.py:165
- Add a test to verify that when an instrument's state is set to DISABLED,
send_to_heroicis not called.
def test_update_instrument_state_calls_out_to_heroic(self, mock_send):
configdb/settings.py:167
- [nitpick] Consider restricting CORS origins instead of allowing all (
CORS_ORIGIN_ALLOW_ALL = True) to reduce exposure to unintended clients.
CORS_ORIGIN_ALLOW_ALL = True
phycodurus
approved these changes
Jun 25, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ment capabilities to HEROIC
I originally thought I would do this entirely with django signals (pre_save for each model), but unfortunately those signals are not triggered with new data when all of the many ManyToMany relationships in models are updated. In order to ensure we only have a single instrument capability update sent to HEROIC per update made in the admin interface or API, I moved the main HEROIC reporting to occur in those two places.
So for LCO's usage, where we update everything in the Admin interface generally, the extra code in the admin.py file will trigger HEROIC reporting when any instrument capability change is made in that interface. Alternatively, in the serializers.py, HEROIC reporting will be triggered by API updates of various parts of the instrument capabilities. The few signal handlers remaining don't involve any M2M relationships so they should work for both API/Admin interface cases and are just to create the instrument/site/telescope and update their properties. I've unit tested the API usage to see that it calls the code to send the update to HEROIC when updates are made through the API. I've manually tested the Admin interface locally as well.