Skip to content

Conversation

@shetzel
Copy link
Contributor

@shetzel shetzel commented May 3, 2021

What does this PR do?

Fixes some deploy output bugs.

  1. When an error happens during the deploy request (like a gack) the command was handling it properly when --json was specified but incorrectly when the progress bar was in use. This was due to ignoring the event handling errors from SDR, so we now throw the error from the event.
  2. Whenever the command was unsuccessful we need to throw from the output formatter so that the framework indicates an error.
  3. The component failures were not being filtered and displayed properly.

@W-8903671@

@shetzel shetzel requested a review from WillieRuemmele May 3, 2021 22:57
protected async deploy(): Promise<void> {
this.isAsync = this.getFlag<Duration>('wait').quantity === 0;
this.isRest = await this.isRestDeploy();
this.log(`*** Deploying with ${this.isRest ? 'REST' : 'SOAP'} API ***`);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toolbelt was displaying this and I think we need to continue doing that due to all the REST/SOAP issues

deploy.onError(() => {
deploy.onError((error: Error) => {
this.progressBar.stop();
throw error;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we listen to SDR deploy events like this we have to handle the errors.

Copy link
Contributor

@WillieRuemmele WillieRuemmele left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even did some light QA, looks good

@WillieRuemmele WillieRuemmele merged commit d1bb8be into main May 4, 2021
@WillieRuemmele WillieRuemmele deleted the sh/deploy-output-fixes branch May 4, 2021 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants