Skip to content

Added improved merging of staging dicts #692

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

Merged
merged 2 commits into from
Feb 19, 2025

Conversation

wk9874
Copy link
Collaborator

@wk9874 wk9874 commented Feb 14, 2025

Added deepmerge as a dependency, wrote a custom merge strategy which merges dicts and overrides lists, and replaced _local_data | self._staging with the deep merge.

Chose to override lists as the deduplication for alerts and tags is handled by the run, so dont need to append them. Also decided with Andrew that for metadata it would be better to overwrite, eg:

run.update_metadata({"test": [1,2]})
run.update_metadata({"test": [3,4]})

Should return {"test": [3,4]}, not {"test": [1,2,3,4]}

Closes #691

@wk9874 wk9874 requested a review from kzscisoft February 14, 2025 13:33
@wk9874 wk9874 changed the base branch from dev to v2.0 February 18, 2025 17:10
@wk9874 wk9874 added this to the v2.0.0-rc1 milestone Feb 19, 2025
@kzscisoft kzscisoft merged commit 9b209fc into v2.0 Feb 19, 2025
12 of 14 checks passed
@kzscisoft kzscisoft deleted the wk9874/update_metadata_tags_offline branch February 19, 2025 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update metadata and tags wont work in Offline mode
2 participants