Skip to content

Allow specific exception types to be retried or not retried#189

Merged
domesticmouse merged 3 commits intogooglemaps:masterfrom
amyboyd:fail-fast-over-limit
Oct 11, 2016
Merged

Allow specific exception types to be retried or not retried#189
domesticmouse merged 3 commits intogooglemaps:masterfrom
amyboyd:fail-fast-over-limit

Conversation

@amyboyd
Copy link
Copy Markdown
Contributor

@amyboyd amyboyd commented Oct 9, 2016

PR #168 hasn't been updated in 2 months so it seems to be abandoned. This PR adds basically the same functionality - but this also allows you to customise which exceptions are retries or not.

By default only OverQueryLimitException will be retried. If you don't want to retry after a OverQueryLimitException (maybe you are in time-sensitive situation where you want to fail fast), you can disable retries with toggleifExceptionIsAllowedToRetry(OverQueryLimitException.class, false). The same goes for other exception types.

@googlebot
Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@amyboyd
Copy link
Copy Markdown
Contributor Author

amyboyd commented Oct 9, 2016

The first commit is cherry-picked from #168 - if they haven't done the CLA, I can remove that commit.

The other commits are authored by me.

@domesticmouse
Copy link
Copy Markdown
Contributor

Hey @googlebot, both @amyboyd and @slukes have signed the CLA, near as I can tell. Thanks!


@Override
public <T, R extends ApiResponse<T>> PendingResult<T> handle(String hostName, String url, String userAgent, Class<R> clazz, FieldNamingPolicy fieldNamingPolicy, long errorTimeout, Integer maxRetries) {
public <T, R extends ApiResponse<T>> PendingResult<T> handle(String hostName, String url, String userAgent, Class<R> clazz, FieldNamingPolicy fieldNamingPolicy, long errorTimeout, Integer maxRetries, ExceptionsAllowedToRetry exceptionsAllowedToRetry) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks like it exceeds the 120 char limit, can you fold it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

public interface RequestHandler {
<T, R extends ApiResponse<T>> PendingResult<T> handle(String hostName, String url, String userAgent, Class<R> clazz, FieldNamingPolicy fieldNamingPolicy, long errorTimeout, Integer maxRetries);
<T, R extends ApiResponse<T>> PendingResult<T> handlePost(String hostName, String url, String payload, String userAgent, Class<R> clazz, FieldNamingPolicy fieldNamingPolicy, long errorTimeout, Integer maxRetries);
<T, R extends ApiResponse<T>> PendingResult<T> handle(String hostName, String url, String userAgent, Class<R> clazz, FieldNamingPolicy fieldNamingPolicy, long errorTimeout, Integer maxRetries, ExceptionsAllowedToRetry exceptionsAllowedToRetry);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please check line length on these two lines.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.


@Override
public <T, R extends ApiResponse<T>> PendingResult<T> handle(String hostName, String url, String userAgent, Class<R> clazz, FieldNamingPolicy fieldNamingPolicy, long errorTimeout, Integer maxRetries) {
public <T, R extends ApiResponse<T>> PendingResult<T> handle(String hostName, String url, String userAgent, Class<R> clazz, FieldNamingPolicy fieldNamingPolicy, long errorTimeout, Integer maxRetries, ExceptionsAllowedToRetry exceptionsAllowedToRetry) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please check line length

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.


@Override
public <T, R extends ApiResponse<T>> PendingResult<T> handlePost(String hostName, String url, String payload, String userAgent, Class<R> clazz, FieldNamingPolicy fieldNamingPolicy, long errorTimeout, Integer maxRetries) {
public <T, R extends ApiResponse<T>> PendingResult<T> handlePost(String hostName, String url, String payload, String userAgent, Class<R> clazz, FieldNamingPolicy fieldNamingPolicy, long errorTimeout, Integer maxRetries, ExceptionsAllowedToRetry exceptionsAllowedToRetry) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Line length

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -0,0 +1,23 @@
package com.google.maps.internal;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add an Apache2.0 copyright header.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@amyboyd amyboyd force-pushed the fail-fast-over-limit branch from 842b5c6 to b9c263d Compare October 11, 2016 08:52
@amyboyd amyboyd force-pushed the fail-fast-over-limit branch from b9c263d to dfbb9de Compare October 11, 2016 08:54
Copy link
Copy Markdown
Contributor

@domesticmouse domesticmouse left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!

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.

4 participants