-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
HADOOP-16080. hadoop-aws does not work with hadoop-client-api #2510
Conversation
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
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.
LGTM, +1
BTW, I think trunk and branch-3.3 can be updated as well in a separate JIRA to avoid the usage of guava as possible. |
Thanks @aajisaka ! yes I do plan to apply this to trunk and branch-3.3. I hope this aligns with the work in HADOOP-17098 (cc @amahussein) which is more general. cc @steveloughran too |
🎊 +1 overall
This message was automatically generated. |
Merged to branch-3.2. I'll create PRs for branch-3.1 and trunk later. Thanks again @aajisaka for the review. |
…#2510). Contributed by Chao Sun
FWIW that checkArgument(varags) does exist in older versions; what's gone wrong is that google added specific overloads for certain non-varargs variant, e.g (String, int) so that stuff built against the newer version isn't going to link. Adding a String.format does make that arg checking a bit more expensive, as it's being calculated even on the correct sequence. We are moving to our own precondition checker because of the guava pain; we should switch to that |
Yes this is a hot fix to make it work with 3.2.2 but totally agree that we should avoid the perf hit perhaps through #2505. In 3.3.x and up this is no longer an issue since we've switched to the third party guava library. |
This does the following:
ListenableFuture
as well asListeningExecutorService
fromSemaphoredDelegatingExecutor
, so that modules such ashadoop-aws
andhadoop-aliyun
can consume the class fromhadoop-client-api
without running into Guava conflicts.checkArgument/checkState/checkNotNull
withString.format
. The former is generally not available in Guava version < 20 so this is to eliminate potential conflicts.