Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
state: include the workload version in the AllWatcher application info. #7563
Conversation
frankban
changed the title from
All watcher: include the workload version in application info.
to
state: include the workload version in the AllWatcher application info.
Jun 27, 2017
rogpeppe
approved these changes
Jun 27, 2017
LGTM with reservations about the way that workload/application version is working in general, but that's not your fault.
| + unit, err := st.Unit(newInfo.Name) | ||
| + if err != nil { | ||
| + return errors.Annotatef(err, "cannot retrieve unit %q", newInfo.Name) | ||
| + } | ||
| // A change in a unit's status might also affect it's application. |
| + workloadVersion, err := unit.WorkloadVersion() | ||
| + if err != nil { | ||
| + return errors.Annotatef(err, "cannot retrieve workload version for %q", unit.Name()) | ||
| + } else if workloadVersion != "" { |
rogpeppe
Jun 27, 2017
Owner
I have to say that this makes me uneasy, because we've essentially got an arbitrary unit being used to provide the workload version for the entire application. I'd've thought that setting the workload version might be similar to setting the application status - only possible for the leader to do.
I guess that's a policy decision though, and given current policy, this seems unavoidable.
frankban
referenced this pull request
in juju/juju-gui
Jun 27, 2017
Closed
Show workload version in the GUI #2600
|
As discussed on IRC showing a single workload version is a lie, but if we are doing this change to exactly match what is already in juju status then we can live with it for now |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
frankban commentedJun 27, 2017
•
Edited 1 time
-
frankban
Jun 27, 2017
Description of change
Include the workload version as part of the application info sent by the all watcher.
See juju/juju-gui#2600
QA steps
Deploy postgresq, run the API calls (WatchAll + Next) to watch the model, and you should see the workload-version as part of the stream when the charm sets it.