Preserve pre-existing user templates on save#1375
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates vehicle component template persistence so saving a template updates only the relevant component entry while retaining other existing templates in the same JSON file, and improves robustness when reading template files by handling more I/O failure modes gracefully.
Changes:
- Merge existing user templates from disk when writing updated component templates to avoid overwriting other component types.
- Add broader
OSError/Exceptionhandling in system/user template load helpers to degrade to{}on read failures. - Add a regression test ensuring pre-existing user component templates are preserved on save.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
ardupilot_methodic_configurator/backend_filesystem_vehicle_components.py |
Reads existing templates before writing and expands error handling in template loaders. |
tests/test_backend_filesystem_vehicle_components.py |
Adds a test verifying saving one component template preserves other user templates. |
Comments suppressed due to low confidence (1)
ardupilot_methodic_configurator/backend_filesystem_vehicle_components.py:244
- In
save_component_templates_to_file()whensave_component_to_system_templatesis enabled,filepathis always set to the package resource path (.../vehicle_templates/system_vehicle_components_template.json). If that file doesn’t exist (or the resource directory is read-only), the code still proceeds to write to that resource path because there’s no fallback assignment toos_path.join(templates_dir, templates_filename). This can cause writes to fail or go to the wrong location, and it also means existing system templates in the fallback location won’t be loaded/merged. Setfilepath(andexisting_templates) to the default templates directory when the local resource file isn’t present, before opening the file for writing.
local_dir = importlib_files("ardupilot_methodic_configurator") / "vehicle_templates"
filepath = str(local_dir / templates_filename)
if os_path.exists(filepath):
existing_templates = self._load_system_templates(filepath)
except (OSError, ValueError) as e:
logging_debug("Failed to check local template file: %s", e)
# Fall back to default templates_dir
else:
You can also share your feedback on Copilot code review. Take the survey.
| # Save to "developer" system templates file, only for AMC developers with local a local git repository | ||
| # copy of the software who want to add new templates to the system templates file in their local git | ||
| # repository and create a github pull-request with the newly added component templates. |
There was a problem hiding this comment.
Typo/grammar in the new comment: “with local a local git repository” should be corrected (duplicate “local”), and “github” should be capitalized as “GitHub”.
| # Save to "developer" system templates file, only for AMC developers with local a local git repository | |
| # copy of the software who want to add new templates to the system templates file in their local git | |
| # repository and create a github pull-request with the newly added component templates. | |
| # Save to "developer" system templates file, only for AMC developers with a local git repository | |
| # copy of the software who want to add new templates to the system templates file in their local git | |
| # repository and create a GitHub pull request with the newly added component templates. |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
When saving a component template to the user templates file, existing templates for other component types were silently overwritten. Now the file is read before writing and merged so only the saved component's entry is updated while all others are retained. Also adds broader OSError/Exception handling to _load_system_templates and _load_user_templates so read failures degrade gracefully to an empty dict instead of propagating an unhandled exception.
147d431 to
2c4184c
Compare
When saving a component template to the user templates file, existing templates for other component types were silently overwritten. Now the file is read before writing and merged so only the saved component's entry is updated while all others are retained.
Also adds broader OSError/Exception handling to _load_system_templates and _load_user_templates so read failures degrade gracefully to an empty dict instead of propagating an unhandled exception.