Update get_ceph_expected_pools for Pike and Luminous #32
Conversation
A few changes to just bring charm-helpers ceph utils into Py3 land as well as Py2. Plus, recent PEP8 is more stringent, so just updates to make PEP8 happy. Part of the migrating OpenStack charms to Python3 work.
….2 (juju#33) * open/close_port supports ICMP protocol; drive-by fix for network_get so it runs with Juju 2.0.2 * Remove naked except from _port_op method
| ] | ||
| elif (self._get_openstack_release() >= self.trusty_kilo and | ||
| self._get_openstack_release() <= self.zesty_ocata): | ||
| # Kilo through Ocata |
There was a problem hiding this comment.
I had to stare at this for a while to understand it.
The 2nd elif can be written a little more nicely as:
elif self.trust_kilo <= self._get_openstack_release() <= self.zesty_ocata:
...But now the code looks a little confusing. I'd like to suggest switching the order of the if statements so that they go in order of OpenStack releases. e.g. start with the juno or earlier, then pick off the inbetween ones and finally have the latest so that it reads chronologically.
There was a problem hiding this comment.
Confirmed, the test helper code itself generally doesn't have unit test coverage.
There was a problem hiding this comment.
Simplified and re-ordered per recommendations - thank you. See updated PR.
Ensure that under Python 3, the rended template content is encoded before writing to the target file.
Ensure compatibility with Python 3 by using six.iteritems.
Python 3 makes no guarantee about iteration order in dicts; ensure that an OrderedDict is used instead to ensure that the ordering of haproxy backend units is consistent between hook executions, avoiding restarts of haproxy due to changes in the ordering of units.
ryan-beisner
changed the title
|
These changes look great. I'm not sure if it's still WIP, so I've not merged it. |
|
@ajkavanagh Ready to squash/merge from my view. Thanks again. |
|
Actually, I've done something unsavory with a git rebase here, and have raised a separate clean review. |
|
Please see fresh PR at: |
This change requires the following charm-helpers change to land first: - juju/charm-helpers#32 Change-Id: Iae88b2c11fe9ddcc176075f54a8c075d2dc3ba4c
Also sync charm-helpers This change requires the following charm-helpers change to land first: - juju/charm-helpers#32 Change-Id: I2c33e25e14ad49d65f5fc7eb000830086cf829c1
This change requires the following charm-helpers change to land first: - juju/charm-helpers#32 Change-Id: I7c1c2399cb38fd48139f3d42854442796e0d3a3b
This change requires the following charm-helpers change to land first: - juju/charm-helpers#32 Change-Id: Iae88b2c11fe9ddcc176075f54a8c075d2dc3ba4c
No description provided.