Removing the hard dependency on Apache HTTP Client - #259
Merged
Conversation
schmidt-sebastian
approved these changes
Mar 13, 2019
schmidt-sebastian
left a comment
Contributor
There was a problem hiding this comment.
My only comment is that you should probably add a NOTICE file similar to http://svn.apache.org/repos/asf/httpcomponents/httpclient/tags/4.3/NOTICE.txt, since that seems to be one of the requirements of the Apache License. Otherwise this looks good, including the simplification thanks to the synchronized locking.
Contributor
Author
|
Added |
Contributor
|
How about migrating to DateTimeFormatter? However it is only supported in Java 8 and forward.A lot of extra code for obsolete and EOL Java version. |
Contributor
Author
|
We cannot use Java 8 at the moment. Current GCP policy requires us to be Java 7 compatible. |
hiranya911
added a commit
that referenced
this pull request
Mar 14, 2019
* Basic framework for HTTP retries * Implementing support for retry-after header * Cleaned up the retry-after processing logic * Moved the status code checking logic * Updated tests * Updated class names and tests * Refactored retry impl and tests * Simplified the retry handler * More tests and docs * Further cleaned up the impl and tests * Decoupled retry initializer from credentials * More code cleanup * Cleaning up tests * Not calling any retry code when RetryConfig = null * Added an option to enable/disable retries on IO errors. Added some comments * New test case * Updated some comments; Cleaned up tests * Fixed a typo in a comment * Removing the hard dependency on Apache HTTP Client (#259) * Copied DateUtils source from Apache HC * Updated reference link * Used locks instead of thread locals; Added tests * Added a NOTICE file for third-party code
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
In #255 we used an Apache HTTP client API (
DateUtils) to parse HTTP date headers. We have this library in our classpath at the moment as a transitive dependency of Google API client. But the Google API client is undergoing a refactor, and this dependency will be removed in a future release. In preparation for this, I'm creating our ownDateUtilsclass by copying the related bit of code from the Apache HTTP client.The original implementation in Apache HTTP client uses thread locals to cache the
SimpleDateFormatinstances, and use them in a thread safe manner. I used explicit locking instead, in order to avoid potential memory leaks that may arise from unreleased thread locals (other suggestions welcome).