Rename wsgi-rotation to wsgi-socket-rotation #833
Conversation
dosaboy
force-pushed
the
wsgi-socket-rotation
branch
from
be0b4ed
to
d1f47cc
Compare
|
not sure why pep8 is failing here, it works fine when i run locally. i have rebase and updated. |
|
LGTM - thanks for the changes |
|
@dosaboy thanks for the patch. Please could you add to the description above why these changes are necessary and, also, as it is will cause a corresponding change in charms (the config var is changing), what charms it is affecting. At the moment, this looks like a breaking change, but it's not possible to see what is breaking. Thanks! |
| enable_wsgi_rotation = config('wsgi-rotation') | ||
| if enable_wsgi_rotation is None: | ||
| enable_wsgi_rotation = True | ||
| enable_wsgi_socket_rotation = config('wsgi-socket-rotation') |
There was a problem hiding this comment.
This causes a potentially breaking change, albeit only on master branch charms. Please could you link to the bug or gerrit reviews where this will be consumed? Thanks.
Commit 996f241 added support for new config option 'wsgi-rotation' but that name should have been 'wsgi-socket-rotation' in order to have a 1:1 relation with the apache config it changes. The following patches that implement this config are currently blocked until this lands so that they can be synced before merge: * https://review.opendev.org/c/openstack/charm-ceilometer/+/887793 * https://review.opendev.org/c/openstack/charm-cinder/+/886356 * https://review.opendev.org/c/openstack/charm-glance/+/886376 * https://review.opendev.org/c/openstack/charm-keystone/+/886377 * https://review.opendev.org/c/openstack/charm-nova-cloud-controller/+/885836 * https://review.opendev.org/c/openstack/charm-openstack-dashboard/+/886373 Related-Bug: #2021550
dosaboy
force-pushed
the
wsgi-socket-rotation
branch
from
d1f47cc
to
1a90eb0
Compare
|
@ajkavanagh thanks for the review. I have addressed your points in the commit msg. This is not a breaking change since the patches to implement it in the charms are blocked on this landing first. |
No description provided.