Merge lp:~frankban/juju-deployer/missing-units into lp:juju-deployer
Proposed by
Francesco Banconi
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | David Britton | ||||
Approved revision: | 142 | ||||
Merged at revision: | 140 | ||||
Proposed branch: | lp:~frankban/juju-deployer/missing-units | ||||
Merge into: | lp:juju-deployer | ||||
Diff against target: |
244 lines (+59/-43) 7 files modified
deployer/service.py (+4/-0) deployer/tests/base.py (+2/-0) deployer/tests/test_data/stack-placement-invalid-subordinate.yaml (+11/-0) deployer/tests/test_deployment.py (+27/-20) deployer/tests/test_diff.py (+4/-6) deployer/tests/test_guiserver.py (+6/-8) deployer/tests/test_importer.py (+5/-9) |
||||
To merge this branch: | bzr merge lp:~frankban/juju-deployer/missing-units | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
David Britton (community) | Approve | ||
Brad Crittenden (community) | code | Approve | |
Madison Scott-Clary (community) | Approve | ||
Review via email:
|
Commit message
Disallow deploying a bundle when a service unit placement points to a subordinate service.
Description of the change
Disallow deploying a bundle when a service unit placement points to a subordinate service.
Without this kind of validation, the bundle would still fail later with an unhelpful KeyError: 'units'.
The big assumption here is that we want the deployer to fail in such cases. Do we? If not, what's the correct behavior?
To post a comment you must log in.
LGTM, thanks for the fix. Per IRC, I believe that the placement directive should specify the unit on which the subordinate is deployed, rather than the subordinate unit itself, since we can't guarantee the order of subordinate deployments, thus can't guarantee which machine it would wind up on.