-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-19343. Add Google Cloud Storage Connector #7656
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-19343. Add Google Cloud Storage Connector #7656
Conversation
Add implementation for create() API
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -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.
Hello @arunkumarchacko . Looks like this is going in the right direction overall! I entered a few questions regarding the build process.
hadoop-project/pom.xml
Outdated
@@ -86,7 +86,7 @@ | |||
|
|||
<!-- Protobuf version for backward compatibility --> | |||
<!-- This is used in hadoop-common for compilation only --> | |||
<protobuf.version>2.5.0</protobuf.version> | |||
<protobuf.version>3.25.3</protobuf.version> |
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.
This was intentionally kept at a lower version for backward-compatibility with other Hadoop RPC clients. HADOOP-17046 has more background. I'm not sure it's safe to upgrade this now. Internally, Hadoop gets its protobuf as a shaded dependency from the hadoop-thirdparty project:
https://github.com/apache/hadoop-thirdparty
Can we use that?
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.
GCS client needs this protobuf version to function. Since the dependency is from GCS client, I do not think we can use hadoop-thridparty.
Thank you for the feedback. I will see if there is any alternate way to resolve this.
hadoop-tools/hadoop-gcp/pom.xml
Outdated
</build> | ||
</profile> | ||
|
||
<!-- Turn on scale tests--> |
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.
Does the pom.xml have some stuff cloned from hadoop-tools/hadoop-aws/pom.xml that isn't relevant for this module?
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.
Yes, this is cloned from hadoop-aws/pom.xml. I kept it since we plan to have parity with other clients. I will remove this for now and add when we actually add those tests.
Fix some spotbug issues
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -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.
+1. I'll commit this to the feature branch shortly.
Closes #7656 Signed-off-by: Chris Nauroth <cnauroth@apache.org>
I have committed this to the feature branch. @arunkumarchacko , thank you for this initial contribution! |
Closes #7656 Signed-off-by: Chris Nauroth <cnauroth@apache.org>
Closes #7656 Signed-off-by: Chris Nauroth <cnauroth@apache.org>
Closes #7656 Signed-off-by: Chris Nauroth <cnauroth@apache.org>
Closes #7656 Signed-off-by: Chris Nauroth <cnauroth@apache.org>
Closes #7656 Signed-off-by: Chris Nauroth <cnauroth@apache.org>
Add implementation for create() API
Description of PR
How was this patch tested?
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?