fix: prevent adding cameras with invalid model names (#1162)#1165
fix: prevent adding cameras with invalid model names (#1162)#1165COZYTRICKSTER wants to merge 3 commits intoopen-edge-platform:release-2025.2from
Conversation
tdorauintc
left a comment
There was a problem hiding this comment.
@COZYTRICKSTER thank you for your contribution.
For the sake of code maintainability and reliability, I would advice to try to reuse existing code for validation. Please see my comments.
manager/src/django/forms.py
Outdated
| from manager.ppl_generator.model_chain import CameraChainOperations | ||
|
|
||
| # Load model config | ||
| model_config_path = Path(os.environ.get('MODEL_CONFIGS_FOLDER', '/models/model_configs')) / 'model_config.json' | ||
| 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)}") | ||
|
|
||
| # Extract model names from chain | ||
| chain_parts = camerachain.replace(CameraChainOperations.PARALLEL.value, CameraChainOperations.SEQUENTIAL.value) | ||
| model_names = [part.split('=')[0].strip() for part in chain_parts.split(CameraChainOperations.SEQUENTIAL.value) if part.strip()] | ||
|
|
||
| # Validate models exist | ||
| missing = [m for m in model_names if m not in model_config] | ||
| if missing: | ||
| missing_str = ', '.join(f"'{m}'" for m in missing) | ||
| available = ', '.join(f"'{m}'" for m in sorted(model_config.keys())) | ||
| raise ValidationError(f"Error adding camera: Model(s) {missing_str} not found in model config file. Available models: {available}") | ||
|
|
There was a problem hiding this comment.
I would suggest to address this issue by reusing existing code in one of the following ways:
- Reusing the function
def parse_model_chain(model_chain: str, models_folder: str, model_config: dict)to validatecamerachain. - Reusing the function
generate_pipeline_string_from_dictto validate the entire form, similarly as it is done here. Since the model config falls back to default if empty, it should also handle the new camera form with limited number of fields.
There was a problem hiding this comment.
@tdorauintc Thank you for the review and suggestions!
I analyzed both options:
-
Option 1 (
parse_model_chain): Works well for validatingcamerachainin isolation.
However, since it buildsInferenceNodeobjects one by one and fails fast on the first
missing model, it would only report the first missing model.
For example, forretail+pv2000+pv20where the last two are missing, onlypv2000is reported. -
Option 2 (
generate_pipeline_string_from_dict): Ultimately calls the sameparse_model_chain
internally, so has the same fail-fast behavior.
I have a concern: the current fix collects all missing models at once and reports them in a single error message,
which is slightly better UX.
I'm happy to refactor to use parse_model_chain as suggested if the fail-fast behavior is acceptable.
Could you confirm your preference? I'll proceed with the refactor once I hear back.
There was a problem hiding this comment.
@COZYTRICKSTER fail-fast behavior is acceptable, please refactor with the 1st option as you suggested. Reusing existing parser parse_model_chain makes the implementation future-proof in case we change syntax some time in future, this brings significant benefits.
There was a problem hiding this comment.
@tdorauintc Thanks for the clarification!
I have refactored the validation to reuse parse_model_chain() as suggested.
The latest commit updates CamCreateForm.clean_camerachain() accordingly.
Please let me know if any further adjustments are needed.
Replace manual model validation in CamCreateForm.clean_camerachain() with parse_model_chain() from ppl_generator. This aligns validation with the existing pipeline parser and ensures future compatibility with potential chain syntax changes. Also includes minor indentation fixes.
There was a problem hiding this comment.
Pull request overview
Adds server-side validation to prevent cameras being created with a camerachain that references models not present in the configured model-config, avoiding “camera exists but nothing processes/tracks” states.
Changes:
- Add
CamCreateForm.clean_camerachain()validation that parses the model chain againstmodel_config.jsonand rejects invalid models. - Override
CamCreateView.form_invalid()to return HTTP 400 on validation failure.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| manager/src/django/forms.py | Adds camerachain validation by loading model-config and calling parse_model_chain() during camera creation. |
| manager/src/django/views.py | Returns 400 status for invalid camera-create form submissions. |
| model_config_path = Path(os.environ.get('MODEL_CONFIGS_FOLDER', '/models/model_configs')) / 'model_config.json' | ||
| 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) |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>


📝 Description
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 validation error.✨ Type of Change
🧪 Testing Scenarios
Tested locally by attempting to add a camera with both valid and invalid model names. Verified that:
✅ Checklist