-
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-17402. Add GCS config to the core-site #2638
Conversation
hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
Outdated
Show resolved
Hide resolved
e18c220
to
8b4ca91
Compare
Assuming the gcs init code initialises all its properties with meaningful defaults, I'd rather leave out everything gcs-related except for the base gcs & viewfs bindings, and that list of excluded properties. putting it into core-default means that when the gcs team change any default value in their own code, the FS will still be inited with whatever was the default when that version of hadoop was built. let's keep this to a minimum |
8b4ca91
to
16a487e
Compare
@steveloughran sounds good, updated in 16a487e |
@@ -142,6 +141,7 @@ public void initializeMemberVariables() { | |||
xmlPropsToSkipCompare.add("fs.viewfs.overload.scheme.target.webhdfs.impl"); | |||
xmlPropsToSkipCompare.add("fs.viewfs.overload.scheme.target.wasb.impl"); | |||
xmlPropsToSkipCompare.add("fs.viewfs.overload.scheme.target.swift.impl"); | |||
xmlPropsToSkipCompare.add("fs.viewfs.overload.scheme.target.gs.impl"); |
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.
Seems like this list should be sorted alphabetically?
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.
@medb I can certainly change this, but it's already not sorted due to swift
.
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.
yeah, but lets try not to make things worse. It's like how we try and follow ordering/layout rules on imports. Strive to to better than your predecessors
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.
@steveloughran done, also fixed the swift
order.
<property> | ||
<name>fs.viewfs.overload.scheme.target.gs.impl</name> | ||
<value>com.google.cloud.hadoop.fs.gcs.GoogleHadoopFS</value> | ||
<description>The HttpFileSystem for view file system overload scheme |
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.
nit: change "HttpFileSystem" to "Google Cloud Storage file system"
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.
@jojochuang done.
16a487e
to
9c4af4d
Compare
9c4af4d
to
838adbb
Compare
838adbb
to
a57700c
Compare
🎊 +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
Merged to trunk. @ravwojdyla : if you cherrypick this to branch-3.3 and submit a new PR there, I'll merge as soon as yetus is happy. Leaving the JIRA open for now thanks! |
Contributed by Rafal Wojdyla
@steveloughran thanks! Done in: #3180. |
Contributed by Rafal Wojdyla
Contributed by Rafal Wojdyla
HADOOP-17402.
FYI @steveloughran , @medb