-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
[FLINK-1266] Generalize DistributedFileSystem implementation #268
Conversation
|
||
WordCount.main(new String[]{input, output}); | ||
|
||
// List<Integer> files = client.listFiles("/", true); |
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.
Could we remove the commented lines here?
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. Its not too hard to come up with the code again if one wants to enhance / debug the test case.
I'll remove the comments.
Looks nice! Since you touched the documentation of the connectors anyways, can you replace the section on MongDB with a reference to this code: https://github.com/okkam-it/flink-mongodb-test Maybe add a small (manual) TOC at the top (or put MongoDB before Azure so both are visible initially) Otherwise, +1 to merge. |
<dependency> | ||
<groupId>org.eclipse.jetty</groupId> | ||
<artifactId>jetty-util</artifactId> | ||
<version>7.6.8.v20121106</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 may conflict with the jetty version used by flink-runtime for the web info server
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.
I will double check what maven makes out of these two versions.
But the Jetty version is test
scoped and should not influence the regular build.
it also seems that I forgot to scope the regular tachyon dependency to test
.
This pull request moves the hadoop transitive dependency exclusions to the dependency management. Have you checked that this works properly? I ran into an issue there a while back where it did not properly work. Cannot quite recall why, though. |
Yes, I found it very inconvenient to copy paste the exclusions everywhere. I will check if the exclusions still work correctly. |
to HadoopFileSystem wrapper, which supports all subclasses of org.apache.hadoop.fs.FileSystem. This allows us to let users use all file systems with support for HDFS. The change has been tested with Tachyon, Google Cloud Storage Hadoop Adapter and HDFS. The change also cleans up the Hadoop dependency exclusions.
Update: I fixed a major bug with the "2.0.0-alpha" version on the I also verified that the dependency exclusions in the |
Thanks, that looks good. |
This closes apache#268
to HadoopFileSystem wrapper, which supports all subclasses of
org.apache.hadoop.fs.FileSystem
.This allows us to let users use all file systems with support for HDFS.
The change has been tested with Tachyon, Google Cloud Storage Hadoop Adapter and HDFS. (I also contributed again to Tachyon)
The change also cleans up the Hadoop dependency exclusions.
I also found a nice little "bug" in the Instance class.