Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Remove the broken status history squashing. #8144
Conversation
|
So this has a small issue if you try to trigger the changes via 'juju run' instead of via 'update-status' hooks. To start with, it is very nice to use the right log formatter, because now the status states are also colored (maintenance is yellow, executing/idle are green). Oddly "waiting" is not colored, and stands out. However, if you use juju-run, this is what I see:
vs with juju-2.3-beta3
Now, it probably isn't 'standard' to use juju-run, but it is something that ends up collapsing a little bit better. Now, if I just show the workload status: The new code does what you expect, in that it doesn't trigger anything for repeated 'status-set' calls, and I do see the timestamp on "juju status --format=yaml" shows the latest status while "show-status-log" shows the first status.
vs
$ juju status --format=yaml t/0
$ juju show-status-log t/0 --type=workload
$ juju show-status-log t/0
$ /usr/bin/juju show-status-log t/0
$ juju debug-log --include t/0 --replay --no-tail | grep "executing.*run update-status"
|
jameinel
approved these changes
Nov 28, 2017
I'm pretty convinced that this is better than what we have, so I'll merge.
I do think we have some bugs remaining around 'update-status' in that the first hook didn't even fire for 20 minutes, and then setting 'update-status-hook-interval' to 1min had no effect.
| @@ -30,8 +32,15 @@ func NewStatusHistoryCommand() cmd.Command { | ||
| return modelcmd.Wrap(&statusHistoryCommand{}) | ||
| } | ||
| +// HistoryAPI is the API surface for the show-status-log command. | ||
| +type HistoryAPI interface { |
jameinel
Nov 28, 2017
Owner
Given the commands are now "show-status-log" rather than "status-history" likely we should be updating naming to match. I certainly mistype the command fairly often because I'm thinking the wrong thing.
Anyway, not something that needs to be fixed here.
|
$$merge$$ |
|
Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju |
howbazaar commentedNov 28, 2017
The squashing of matching status history values is broken and is removing some values that it shouldn't be. The whole point of squashing the status history values was to remove the repeated update-status calls, and the idle following them. Given that we are no longer recording the update-status calls, the squashing should be unnecessary.
QA steps
Bug reference
https://bugs.launchpad.net/juju/+bug/1642031