-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-19081. Move some hadoop-common code to new hadoop-ftp module #6693
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
Conversation
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
@steveloughran I'm not sure that this PR is worth pursuing. The FTP classes are referenced in core-defaults.xml So it seems like values would need to be removed from core-defaults.xml. I am not an expert on Hadoop configuration and am unsure what should be done to the configs if we want to continue with this. Hadoop HA would have similar issues if we wanted to move the HA code out of hadoop-common. |
no, we can stick stuff in core-default. there's s3a options there, etc. |
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 do think this is good, as it cuts down the dependencies.
I think in the past some of the hdfs failover code uses jsch, but think that zk replaced all of that a long time ago. still worth warning hdfs and yarn about though.
I'd propose moving to hadoop-tools
@steveloughran sure, the classes can move to an existing jar. Would you be able to suggest what to do about core-default.xml? The FTP related props will probably have to move too. The hadoop-common config tests break because I've moved the code. |
Just passing by-
AFAIK there are ways to exclude some configs in those tests, putting in some HashMap, like |
|
💔 -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. |
🎊 +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.
core move looks good; we will have to tag as incompatible to
-anyone who wants the things on their CP
-anyone who wants it in common/lib and to use the ftp/sftp clients
without doing things there.
Do we still want to stitch it into the hadoop classpath?
@@ -0,0 +1,337 @@ | |||
# Licensed to the Apache Software Foundation (ASF) under one |
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 this class get into the JAR? if so it should go into test/resources
We're closing this stale PR because it has been open for 100 days with no activity. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
Description of PR
HADOOP-19081 - try to reduce hadoop-common so that it doesn't have so many dependencies
WIP
How was this patch tested?
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?