Juju Charms Collection
glance package

Merge lp://staging/~ajkavanagh/charms/trusty/glance/use-assess-status-lp1531767 into lp://staging/~openstack-charmers-archive/charms/trusty/glance/next

Proposed by Alex Kavanagh on 2016-01-12

Status:	Merged
Merged at revision:	162
Proposed branch:	lp://staging/~ajkavanagh/charms/trusty/glance/use-assess-status-lp1531767
Merge into:	lp://staging/~openstack-charmers-archive/charms/trusty/glance/next
Diff against target:	272 lines (+104/-52) 5 files modified actions/actions.py (+10/-5) hooks/glance_relations.py (+3/-6) hooks/glance_utils.py (+34/-0) unit_tests/test_actions.py (+23/-41) unit_tests/test_glance_utils.py (+34/-0)
To merge this branch:	bzr merge lp://staging/~ajkavanagh/charms/trusty/glance/use-assess-status-lp1531767
Related bugs:	Link a bug report

Reviewer	Date Requested	Status
David Ames (community)		Approve on 2016-01-19
Liam Young (community)	2016-01-12	Needs Fixing on 2016-01-13
Review via email: mp+282343@code.staging.launchpad.net

Description of the change

Change to the implementation of pause and resume actions such that they use a new function called assess_status() which (currently) only checks if the unit is_paused() -- which is also new.

Note that it doesn't (as yet?) change any of the other status_set() functions which means this is only a partial solution.

Revision history for this message

Liam Young (gnuoy) wrote on 2016-01-13:

This look good, just a couple of comments inline

review: Needs Fixing

Revision history for this message

uosci-testing-bot (uosci-testing-bot) wrote on 2016-01-13:

charm_lint_check #17246 glance-next for ajkavanagh mp282343
LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/17246/

Revision history for this message

uosci-testing-bot (uosci-testing-bot) wrote on 2016-01-13:

charm_unit_test #16113 glance-next for ajkavanagh mp282343
UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/16113/

Revision history for this message

David Ames (thedac) wrote on 2016-01-13:

As the purpose of the MP is to implement assess_status() we need to make sure it runs after every hook.

Currently in glance_relations.py set_os_workload_status is called which will clobber any paused status. This should be replaced with a call to asssess_status() and as Liam points out assess_status should ultimately call set_os_workload_status.

if __name__ == '__main__':
    try:
        hooks.execute(sys.argv)
    except UnregisteredHookError as e:
        juju_log('Unknown hook {} - skipping.'.format(e))
    set_os_workload_status(CONFIGS, REQUIRED_INTERFACES,
                           charm_func=check_optional_relations)

review: Needs Fixing

Revision history for this message

uosci-testing-bot (uosci-testing-bot) wrote on 2016-01-13:

charm_amulet_test #8752 glance-next for ajkavanagh mp282343
AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/8752/

Revision history for this message

Alex Kavanagh (ajkavanagh) wrote on 2016-01-14:

> As the purpose of the MP is to implement assess_status() we need to make sure
> it runs after every hook.
>
> Currently in glance_relations.py set_os_workload_status is called which will
> clobber any paused status. This should be replaced with a call to
> asssess_status() and as Liam points out assess_status should ultimately call
> set_os_workload_status.
>
> if __name__ == '__main__':
> try:
> hooks.execute(sys.argv)
> except UnregisteredHookError as e:
> juju_log('Unknown hook {} - skipping.'.format(e))
> set_os_workload_status(CONFIGS, REQUIRED_INTERFACES,
> charm_func=check_optional_relations)

Yep, ok. I'll do an search of all the set_os_workload_status() calls and see if they would be better as assess_status() -- i.e. just to check that it won't clobber a pause status as you say.

lp://staging/~ajkavanagh/charms/trusty/glance/use-assess-status-lp1531767 updated on 2016-01-14

161. By Alex Kavanagh on 2016-01-14

Change set_os_workload_status() call in hooks/glance_relations.py to assess_status(...) call

This is so that invoking glance_relations.py doesn't clobber a user set 'paused' state.

Revision history for this message

uosci-testing-bot (uosci-testing-bot) wrote on 2016-01-14:

charm_lint_check #17318 glance-next for ajkavanagh mp282343
LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/17318/