Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(318)

Issue 54950046: jujud: NotProvisioned only fatal on login

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 3 months ago by fwereade
Modified:
10 years, 3 months ago
Reviewers:
mp+203559, rog
Visibility:
Public.

Description

jujud: NotProvisioned only fatal on login fixes very flaky tests https://code.launchpad.net/~fwereade/juju-core/fix-flaky-tests-2/+merge/203559 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -5 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/jujud/agent.go View 3 chunks +12 lines, -4 lines 2 comments Download
M cmd/jujud/agent_test.go View 3 chunks +32 lines, -1 line 0 comments Download

Messages

Total messages: 3
fwereade
Please take a look.
10 years, 3 months ago (2014-01-28 14:37:25 UTC) #1
rog
LGTM https://codereview.appspot.com/54950046/diff/1/cmd/jujud/agent.go File cmd/jujud/agent.go (right): https://codereview.appspot.com/54950046/diff/1/cmd/jujud/agent.go#newcode105 cmd/jujud/agent.go:105: if err == worker.ErrTerminateAgent { || isUpgraded(err) { ...
10 years, 3 months ago (2014-01-28 14:44:12 UTC) #2
fwereade
10 years, 3 months ago (2014-01-28 15:11:49 UTC) #3
On 2014/01/28 14:44:12, rog wrote:
> LGTM
> 
> https://codereview.appspot.com/54950046/diff/1/cmd/jujud/agent.go
> File cmd/jujud/agent.go (right):
> 
> https://codereview.appspot.com/54950046/diff/1/cmd/jujud/agent.go#newcode105
> cmd/jujud/agent.go:105: if err == worker.ErrTerminateAgent {
> || isUpgraded(err) {
> 
> ?
> 
> https://codereview.appspot.com/54950046/diff/1/cmd/jujud/agent.go#newcode176
> cmd/jujud/agent.go:176: if params.IsCodeNotProvisioned(err) {
> || params.IsCodeUnauthorized(err) {
> ?

Cheers -- but I feel it's a fragment more readable without the
condition-glomming.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b