fix(web): invoke pipeline config exception handling #1831
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The key is that gate used to respond with 400 even the request wasn't bad. With this PR, the response better reflects what's actually going on so callers can better decide when to retry. |
kirangodishala
force-pushed
the
invokePipelineConfig-exception-handling
branch
2 times, most recently
from
9a5614a to
24855d9
Compare
…icationRefreshInitialDelayMs which provides an initial delay in milliseconds for the thread that refreshes the applications cache in ApplicationService It's primarily to facilitate testing, but it seems reasonable someone might want use it production to keep things quiet at startup.
…ipelineConfig as determined by wiremock responses from front50 and orca (InvokePipelineConfigTest) and when PipelineService.trigger throws an exception (PipelineControllerTest)
…ig bubble up so gate's http response code more closely reflects what happened
to prepare for changes to the constructor logic
…nses from PipelineController.invokePipelineConfig by handling RetrofitErrors with SpinnakerRetrofitErrorHandler As part of this PipelineController.invokePipelineConfig no longer logs its own message for RetrofitErrors. There's some loss of information with this, as the initiator of the downstream communication is from no longer clear. A subsequent commit restores this.
…lineConfig so it's clear which operation is failing. This improves both logging and gate's http response. As part of this, remove the no-op catch and throw for NotFoundException. With no other more general catch block, this code isn't necessary.
Uses new kork methods to clean up some error handling and custom exception message creation.
kirangodishala
force-pushed
the
invokePipelineConfig-exception-handling
branch
from
24855d9 to
2d60d0d
Compare
dbyron-sf
approved these changes
Sep 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
demonstrate current behavior of PipelineController.invokePipelineConfig as determined by wiremock responses from front50 and orca (InvokePipelineConfigTest) and when PipelineService.trigger throws an exception (PipelineControllerTest)
let exceptions during PipelineController.invokePipelineConfig bubble up so gate's http response code more closely reflects what happened
change PipelineController to use constructor autowiring to prepare for changes to the constructor logic
include information from downstream services in error responses from PipelineController.invokePipelineConfig by handling RetrofitErrors with SpinnakerRetrofitErrorHandler
As part of this PipelineController.invokePipelineConfig no longer logs its own message for RetrofitErrors. There's some loss of information with this, as the initiator of the downstream communication is from no longer clear. A subsequent commit restores this.
chain Spinnaker*Exceptions in PipelineController.invokePipelineConfig so it's clear which operation is failing. This improves both logging and gate's http response.
As part of this, remove the no-op catch and throw for NotFoundException. With no other more general catch block, this code isn't necessary.
introduce new configuration property services.front50.applicationRefreshInitialDelayMs which provides an initial delay in milliseconds for the thread that refreshes the applications cache in ApplicationService It's primarily to facilitate testing, but it seems reasonable someone might want use it production to keep things quiet at startup.