Retry Conflicts for AnotherOperationInProgress #3653
Conversation
danielrbradley
force-pushed
the
retry-conflict
branch
from
172e26c
to
6a41396
Compare
|
Note that the feature toggle variable has been renamed to |
This code path still seems to use the variable I'm referencing: |
Does the PR have any schema changes?Looking good! No breaking changes found. |
Yes, the rename is in a PR #3630 that will be merged shortly (I hope). |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3653 +/- ##
==========================================
- Coverage 59.07% 57.43% -1.64%
==========================================
Files 70 70
Lines 11102 11131 +29
==========================================
- Hits 6558 6393 -165
- Misses 4041 4257 +216
+ Partials 503 481 -22 ☔ View full report in Codecov by Sentry. |
Ok, happy to wait for that to go in, update this, then merge. |
The new azclient allows us to pass a callback function to decide if we want to retry a request. The default behaviour is to only retry a specific set of status codes. This extends that behaviour to also check Conflict (409) and retry if the "error.code" field is "AnotherOperationInProgress". We're avoiding retrying all conflict status codes because some won't be fixed with a retry, so we're targeting the specific case instead. Tested this both manually stepping through the code with a debugger and also created an integration test which previously failed but now passes when configured to use the new azclient instead of autorest client. To access this improved behaviour right now, set the environment variable `PULUMI_USE_AUTOREST=false`. This will become the default in the future, but is currently in testing behind a feature flag. Fixes #903 Rework to make testing easier - Deserialize into concrete struct rather than maps to avoid possible panics. - 100% coverage of shouldRetryConflict and makes ShouldRetry trivial - which is tested via the integration test. Use azcore runtime.NewResponseError - Handles XML variants of errors too. Use PULUMI_ENABLE_AZCORE_BACKEND instead of PULUMI_USE_AUTOREST
danielrbradley
force-pushed
the
retry-conflict
branch
from
87fe409
to
f37c6be
Compare
|
Squashed, rebased and switch PULUMI_USE_AUTOREST to PULUMI_ENABLE_AZCORE_BACKEND in the test |
|
This PR has been shipped in release v2.69.0. |
The new azclient allows us to pass a callback function to decide if we want to retry a request. The default behaviour is to only retry a specific set of status codes. This extends that behaviour to also check Conflict (409) and retry if the "error.code" field is "AnotherOperationInProgress".
We're avoiding retrying all conflict status codes because some won't be fixed with a retry, so we're targeting the specific case instead.
Tested this both manually stepping through the code with a debugger and also created an integration test which previously failed but now passes when configured to use the new azclient instead of autorest client.
To access this improved behaviour right now, set the environment variable
PULUMI_ENABLE_AZCORE_BACKEND=true. This will become the default in the future, but is currently in testing behind a feature flag.This should address #903 which we will close once we've defaulted to using the new azcore implementation: