Code review comment for lp:~sseman/juju-ci-tools/model-change-watcher-py3

Revision history for this message
Aaron Bentley (abentley) wrote :

> Thanks for reviewing. Mostly, I updated the code per you suggestion. Some will update on my follow up branch. See inline comments.
>> - for model in self.models.values():
>> + for model in list(self.models.values()):
>
> I changed it back. Maybe this is something added for debugging an issue.

I wrote that code for a reason. For you. If you don't understand it, you should ask me.

Specifically, the length of self.models.values changes during iteration. You will see a warning about that when you run the tests. listifying self.models.values fixes that problem.

The length of self.models.values changes because FakeEnvironmentState.destroy_model does:
del self.controller.models[self.name]

review: Needs Fixing

« Back to merge proposal