Merge lp:~sinzui/charmworld/stale-metadata into lp:~juju-jitsu/charmworld/trunk
Status: | Merged |
---|---|
Approved by: | Brad Crittenden |
Approved revision: | 313 |
Merged at revision: | 310 |
Proposed branch: | lp:~sinzui/charmworld/stale-metadata |
Merge into: | lp:~juju-jitsu/charmworld/trunk |
Diff against target: |
112 lines (+34/-17) 3 files modified
charmworld/jobs/ingest.py (+7/-6) charmworld/jobs/tests/test_scan.py (+26/-9) charmworld/testing/factory.py (+1/-2) |
To merge this branch: | bzr merge lp:~sinzui/charmworld/stale-metadata |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Brad Crittenden (community) | code | Approve | |
Review via email: mp+175126@code.launchpad.net |
Commit message
Always update the historic charm data with the new metadata and config data.
Description of the change
RULES
Pre-
* New metadata like requires and provides are not appearing in
manage.
* The scan_charm function gathers the new metadata (and config data)
as dict, then wrongly updates the dict with the historic charm data.
The assumption was that the merger of the two dicts was safe because
they never share the same keys.
* This has not been true for several weeks. A change to the ingest
process always starts with the historic charm data to ensure
failures during ingestion have some sane data to save.
* The simple fix is to reverse the update of the dicts, so that the
new data overwrites the old data.
ADDENDUM
* Aaron remarked that process_charm() is mutating the charm_data, but
we want to instead use the returned dict. We want to work with a
copy of the data to minimise the mutations to the dict --be safe.
* Update process_charm() to work with a copy of the charm_data
* Update callsites to always use the returned charm_data.
QA
* Wait for ingestion to run on staging.
* Verify that the mysql charm requires 3 interfaces instead of 1.
This change looks good.
I was initially concerned that the copy in 'process_charm' was not a deep copy given that the charm is a dict of dicts. But looking at the code, no lower-level dicts are mutated so it should be fine.