Re: [jclouds/jclouds] JCLOUDS-1294: Attempt to retry RetryableErrors in Azure ARM (#1203)
Thanks! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1203#issuecomment-386785419
Re: [jclouds/jclouds] JCLOUDS-1294: Attempt to retry RetryableErrors in Azure ARM (#1203)
Well, i can confirm it fixes the failing tests :clap: I'll run the full suite of LiveTests to see if i find more problems -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1203#issuecomment-386736498
Re: [jclouds/jclouds] JCLOUDS-1294: Attempt to retry RetryableErrors in Azure ARM (#1203)
Closed #1203. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1203#event-1609392221
Re: [jclouds/jclouds] JCLOUDS-1294: Attempt to retry RetryableErrors in Azure ARM (#1203)
Merged to [master](http://git-wip-us.apache.org/repos/asf/jclouds/commit/b29f716a) and [2.1.x](http://git-wip-us.apache.org/repos/asf/jclouds/commit/b144d9f4). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1203#issuecomment-386544321
Re: [jclouds/jclouds] JCLOUDS-1294: Attempt to retry RetryableErrors in Azure ARM (#1203)
andreaturli approved this pull request. +1 thanks -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1203#pullrequestreview-117528611
Re: [jclouds/jclouds] JCLOUDS-1294: Attempt to retry RetryableErrors in Azure ARM (#1203)
@nacx pushed 1 commit. 72b76a4 Address review comments -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/jclouds/jclouds/pull/1203/files/c0db82e6796fde2a2bcdd0e585b3bf9bec10c643..72b76a4e7daee5974e29fb89e1b7948ac3712acd
Re: [jclouds/jclouds] JCLOUDS-1294: Attempt to retry RetryableErrors in Azure ARM (#1203)
andreaturli approved this pull request.
looks good to me and very useful! Thanks!
`AzureRetryableErrorHandlerTest` suite is ok too
Please update https://issues.apache.org/jira/browse/JCLOUDS-1294 accordingly.
> + if (response.getStatusCode() != 429 || isRateLimitError(response)) {
+ return false;
+ }
+
+ try {
+ // Note that this will consume the response body. At this point,
+ // subsequent retry handlers or error handlers will not be able to
read
+ // again the payload, but that should only be attempted when the
+ // command is not retryable and an exception should be thrown.
+ Error error = parseError.apply(response);
+ logger.debug("processing error: %s", error);
+
+ boolean isRetryable =
RETRYABLE_ERROR_CODE.equals(error.details().code());
+ return isRetryable ? super.shouldRetryRequest(command, response) :
false;
+ } catch (Exception ex) {
+ // If we can't parse the error, just assume it is not a retryable
error
would it be worth to add a `logger.warn` that suggests what's happening?
> @@ -30,10 +31,32 @@
@Singleton
public class AzureRateLimitRetryHandler extends RateLimitRetryHandler {
+ private final AzureRetryableErrorHandler retryableErrorHandler;
+
+ @Inject
+ AzureRateLimitRetryHandler(AzureRetryableErrorHandler
retryableErrorHandler) {
+ this.retryableErrorHandler = retryableErrorHandler;
+ }
+
+ @Override
+ protected boolean delayRequestUntilAllowed(HttpCommand command,
HttpResponse response) {
+ if (!isRateLimitError(response)) {
+ // If it is not a rate-limit error but the response is a 429, delegate
[minor] I'd rather move this to the javadoc, as it looks really important
> @@ -330,7 +330,6 @@ public Provisionable get() {
@Override
public boolean apply(final String name) {
checkNotNull(name, "name cannot be null");
-boolean present = false;
thanks for cleaning those `present` booleans up!
> +
+ assertFalse(handler.shouldRetryRequest(command, response));
+ }
+
+ @Test
+ public void testDoesNotRetryWhenErrorNotRetryable() {
+ String nonRetryable =
"{\"error\":{\"code\":\"ReferencedResourceNotProvisioned\",\"message\":\"Not
provisioned\"}}";
+ HttpCommand command = new
HttpCommand(HttpRequest.builder().method("GET").endpoint("http://localhost;).build());
+ HttpResponse response =
HttpResponse.builder().statusCode(429).payload(nonRetryable).build();
+
+ assertFalse(handler.shouldRetryRequest(command, response));
+ }
+
+ @Test
+ public void testRetriesWhenRetryableError() {
+ String nonRetryable =
"{\"error\":{\"code\":\"RetryableError\",\"message\":\"Resource busy\"}}";
shouldn't this be `retryable` ?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1203#pullrequestreview-117521108
[jclouds/jclouds] JCLOUDS-1294: Attempt to retry RetryableErrors in Azure ARM (#1203)
https://issues.apache.org/jira/browse/JCLOUDS-1294 This adds a handler for Azure `RetryableError` codes. Note that there is a second commit in the PR with some small cleanup that is not relevant to this PR. @danielestevez Could you give this branch a try? You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds/pull/1203 -- Commit Summary -- * JCLOUDS-1294: Attempt to retry RetryableErrors in Azure ARM * Cleanup unused variables -- File Changes -- M core/src/main/java/org/jclouds/http/handlers/RateLimitRetryHandler.java (2) M providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/config/AzurePredicatesModule.java (6) A providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Error.java (58) M providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureComputeErrorHandler.java (13) M providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureRateLimitRetryHandler.java (23) A providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureRetryableErrorHandler.java (80) M providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/VaultApiLiveTest.java (23) M providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/VaultApiMockTest.java (3) A providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/handlers/AzureRetryableErrorHandlerTest.java (88) -- Patch Links -- https://github.com/jclouds/jclouds/pull/1203.patch https://github.com/jclouds/jclouds/pull/1203.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1203
