Skip to content
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

bug 1330395 - recursive freeze_values and unfreeze_values #24

Closed
escapewindow opened this issue Nov 1, 2016 · 0 comments · Fixed by #58
Closed

bug 1330395 - recursive freeze_values and unfreeze_values #24

escapewindow opened this issue Nov 1, 2016 · 0 comments · Fixed by #58

Comments

@escapewindow
Copy link
Contributor

We currently have scriptworker.config.freeze_values that takes a dict, and changes any list values to tuples, and any dict values to frozendicts. This is useful to avoid inadvertent changes to runtime config.

This was sufficient as long as the config wasn't nested. That isn't true anymore.

In several places in the tests I do dict(deepcopy(config)) and do a for loop to change all frozendicts to dicts for json serialization. A reverse function, to recursively unfreeze a frozendict, might be useful, as long as it's very carefully used.

We can either write our own recursive freeze_values or copy/modify mozharness' ReadOnlyDict:

Write our own

  • we'll need to freeze from the bottom up, since freezing a top level dict and then modifying its values won't work
  • we'll need to unfreeze from the top down, since trying to unfreeze a child with a locked parent won't work

Copy mozharness' ReadOnlyDict

MPL2, so easy to drop in. We may want to allow for un-freezing, or make sure a deepcopy returns a read-write dict.

I don't expect to get to this before 1.0.0. Filing in case someone else is interested.

@escapewindow escapewindow changed the title recursive freeze_values and unfreeze_values bug 1330395 - recursive freeze_values and unfreeze_values Jan 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant