Update box create step to wait for a successful create operation before allowing the HCP Vagrant post-processor to continue #129
Conversation
chrisroberts
force-pushed
the
bug-box-ready
branch
from
1ef2eda to
37ba383
Compare
There was a problem hiding this comment.
@chrisroberts thanks for putting together a fix so quickly. Overall the added wait looks good to me. I left a comment on the error message as I found them to be a bit cryptic.
When it comes to working with HCP related things we found the errors that come back from the clients tend to lack a call to action. So we like to provide users with verbose error messages and provide them with a call to action if they can retry or do something to resolve the error elsewhere.
| @@ -67,6 +71,48 @@ func (s *stepCreateBox) Run(ctx context.Context, state multistep.StateBag) multi | |||
| } | |||
|
|
|||
| ui.Say(fmt.Sprintf("Created new box: %s", config.Tag)) | |||
| if cresp.Payload == nil || cresp.Payload.Operation == nil { | |||
| state.Put("error", fmt.Errorf("Unable to wait for box to become available - invalid response received.")) | |||
There was a problem hiding this comment.
Is there any action the user can or should take at this point?
From the error message is it not clear to what the user should expect at this point. Has the box been uploaded but is not yet available? Or will the box not be created because of some internal error?
Since we halt the post-processor here it would be good to let the user know if they should retry because a box was not created or if they should check HCP Vagrant for completion.
There was a problem hiding this comment.
A missing payload in the response, or the payload not including the operation shouldn't happen if the response was successful, but we still want to test for the unexpected. In this case, there would be no operation to wait on but the response from the box creation would be successful. Would updating the error message to something like this work: "Unable to wait for box to become available - invalid response received. Please try again."
There was a problem hiding this comment.
In this case, there would be no operation to wait on but the response from the box creation would be successful
If I understand this correctly, if we end up in this case the box would've been created successfully but for some strange reason we got no payload. Is that correct?
If so, I think asking the user the try again is not necessary because the box was created but for some old reason the payload was empty. In this case I do wonder if the error message should read.
Borrowing from you error messaging below
Failure waiting for box to become available. Please check HCP Vagrant for box status, and try again if the box failed to be created.
There was a problem hiding this comment.
Thanks for the internal discussion. I understand the approach and why you are choosing to halt and not continue with a warning to the user. I left a suggestion to the error message but it may not help anymore than what you already have.
I think in the end if the user knows what state the box is in and what they need to do next that should be enough to help them resolve any intermittent connectivity or timeout related blips.
| @@ -67,6 +71,48 @@ func (s *stepCreateBox) Run(ctx context.Context, state multistep.StateBag) multi | |||
| } | |||
|
|
|||
| ui.Say(fmt.Sprintf("Created new box: %s", config.Tag)) | |||
| if cresp.Payload == nil || cresp.Payload.Operation == nil { | |||
| state.Put("error", fmt.Errorf("Unable to wait for box to become available - invalid response received.")) | |||
There was a problem hiding this comment.
In this case, there would be no operation to wait on but the response from the box creation would be successful
If I understand this correctly, if we end up in this case the box would've been created successfully but for some strange reason we got no payload. Is that correct?
If so, I think asking the user the try again is not necessary because the box was created but for some old reason the payload was empty. In this case I do wonder if the error message should read.
Borrowing from you error messaging below
Failure waiting for box to become available. Please check HCP Vagrant for box status, and try again if the box failed to be created.
If the registry box needs to be created, wait for the create operation to finish before allowing the post-processor to continue to the next step.
chrisroberts
force-pushed
the
bug-box-ready
branch
from
37ba383 to
784850a
Compare
nywilken
changed the title
If the registry box needs to be created, wait for the create operation to finish before allowing the post-processor to continue to the next step.
Closes #128