fix: prevent adding cameras with invalid model names (#1162)#1445
Open
Copilot wants to merge 2 commits into
Open
fix: prevent adding cameras with invalid model names (#1162)#1445Copilot wants to merge 2 commits into
Copilot wants to merge 2 commits into
Conversation
Agent-Logs-Url: https://github.com/open-edge-platform/scenescape/sessions/e2110882-d817-4131-9c80-8ebfe3f648eb Co-authored-by: saratpoluri <1325325+saratpoluri@users.noreply.github.com>
Agent-Logs-Url: https://github.com/open-edge-platform/scenescape/sessions/e2110882-d817-4131-9c80-8ebfe3f648eb Co-authored-by: saratpoluri <1325325+saratpoluri@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
saratpoluri
May 20, 2026 00:26
View session
saratpoluri
requested review from
FilipAdamus97,
Irakus,
MikolajKasprzak,
daddo-intel,
dmytroye,
dpitulax,
ltalarcz,
sbelhaik and
tdorauintc
as code owners
saratpoluri
approved these changes
May 20, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an invalid camera configuration path in the manager service by validating the submitted camera model chain against the configured model_config.json during camera creation, rejecting invalid configurations early to avoid “camera exists but no tracking” states.
Changes:
- Add
CamCreateForm.clean_camerachain()validation that loadsmodel-configand validates the chain viaparse_model_chain. - Return HTTP 400 from
CamCreateViewwhen the submitted form is invalid. - Update the camera-create test to expect 400 on invalid submissions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
manager/src/manager/forms.py |
Adds clean_camerachain validation by loading model config and validating the chain. |
manager/src/manager/views.py |
Overrides form_invalid to respond with HTTP 400 on validation errors. |
tests/sscape_tests/cam/test_cam_create.py |
Adjusts expected status code for invalid camera creation POSTs. |
Comment on lines
+195
to
+203
| model_config_path = Path(os.environ.get('MODEL_CONFIGS_FOLDER', '/models/model_configs')) / model_config_filename | ||
| if not model_config_path.is_file(): | ||
| raise ValidationError(f"Model config file not found at {model_config_path}") | ||
|
|
||
| try: | ||
| with open(model_config_path, 'r') as f: | ||
| model_config = json.load(f) | ||
| except (json.JSONDecodeError, IOError) as e: | ||
| raise ValidationError(f"Error reading model config: {str(e)}") |
Comment on lines
18
to
+20
| def test_cam_create_page(self): | ||
| response = self.client.post(reverse('cam_create'), data = {'sensor_id': '100', 'name': 'test_camera', 'scene': 'test_scene'}) | ||
| self.assertEqual(response.status_code, 200) | ||
| self.assertEqual(response.status_code, 400) |
Comment on lines
18
to
+20
| def test_cam_create_page(self): | ||
| response = self.client.post(reverse('cam_create'), data = {'sensor_id': '100', 'name': 'test_camera', 'scene': 'test_scene'}) | ||
| self.assertEqual(response.status_code, 200) | ||
| self.assertEqual(response.status_code, 400) |
Comment on lines
+194
to
+203
| model_config_filename = getattr(self.instance, 'modelconfig', None) or 'model_config.json' | ||
| model_config_path = Path(os.environ.get('MODEL_CONFIGS_FOLDER', '/models/model_configs')) / model_config_filename | ||
| if not model_config_path.is_file(): | ||
| raise ValidationError(f"Model config file not found at {model_config_path}") | ||
|
|
||
| try: | ||
| with open(model_config_path, 'r') as f: | ||
| model_config = json.load(f) | ||
| except (json.JSONDecodeError, IOError) as e: | ||
| raise ValidationError(f"Error reading model config: {str(e)}") |
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.
📝 Description
Applies the changes from #1165 to main.
When adding a camera, the system allowed specifying a model name that does not exist in the configured model list (
model-config). The camera would still be created successfully even though the model was invalid.This could result in a confusing state where:
This change introduces validation during camera creation to ensure that the provided model name exists in
model-config. If the model is not found, the request is rejected with a validation error.Fixes #1162
🔍 Behavior Before / After
Before
model-config.After
model-config, the request is rejected with an appropriate 400 validation error.✨ Type of Change
✅ Changes
manager/src/manager/forms.py: Addedclean_camerachainvalidation method toCamCreateFormthat readsmodel-configand validates the provided model name viaparse_model_chain.manager/src/manager/views.py: Addedform_invalidoverride toCamCreateViewto return HTTP 400 on validation failure.tests/sscape_tests/cam/test_cam_create.py: Updated expected status code from 200 to 400 for invalid camera creation submissions.