-
Notifications
You must be signed in to change notification settings - Fork 289
fix: Enabled automatic HTTP retries for FirebaseProjectManagement #356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
||
// Verify retry support | ||
HttpUnsuccessfulResponseHandler retryHandler = request.getUnsuccessfulResponseHandler(); | ||
assertTrue(retryHandler instanceof RetryHandlerDecorator); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first I kind of want to say we should bake this assertion into the API infrastructure itself instead of directly verifying in a unit test that no one touched the code. But then the OO design I came up with to facilitate this turns out to be impossible due to various classes being final
. I was hoping that we can write a subclass of HttpRequest
called RetryingHttpRequest
that declares at compile time that getUnsuccessfulResponseHandler
will return RetryHandlerDecorator
or a suitable base class, but alas, HttpRequest
is final. (And so is HttpRequestFactory
, which is the other class I wanted to subclass.)
Without the above OO structure, the only other way to structure the test to be more behavioural would be to treat the retrying mechanism completely like a black box, and empirically measure retry intervals and retry counts after forcing an unsuccessful response. This is going to be painfully flaky, so I think what you have is probably for the best.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM. The only comment I left is just an interesting observation.
Enables automatic HTTP retries for the
FirebaseProjectManagement
API. We disable retries for unit tests for faster execution of tests.RELEASE NOTE: Project management APIs in the
FirebaseProjectManagement
class now automatically retries operations that fail due to retry-eligible HTTP errors.