-
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-17880. Build Hadoop on Centos 7 #3535
HADOOP-17880. Build Hadoop on Centos 7 #3535
Conversation
(!) A patch to the testing environment has been detected. |
@GauthamBanasandra @iwasakims I move patches from pr #3349 here, and close #3349, and do the following:
please review again, thanks. |
💔 -1 overall
This message was automatically generated. |
Thanks for PR @ZhendongBai. Could you please take a look at the pylint warnings? |
(!) A patch to the testing environment has been detected. |
@GauthamBanasandra Ok, and for more compatibility, use |
💔 -1 overall
This message was automatically generated. |
(!) A patch to the testing environment has been detected. |
💔 -1 overall
This message was automatically generated. |
(!) A patch to the testing environment has been detected. |
💔 -1 overall
This message was automatically generated. |
Platform package dependency resolver for building Apache Hadoop. | ||
""" | ||
|
||
from __future__ import print_function |
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.
Where is this package getting used? I'm not able to locate it 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.
@GauthamBanasandra This line was used to suppress pylint warning - Unnecessary parens after 'print' keyword
, the solution was found at https://stackoverflow.com/questions/28694380/pylint-says-unnecessary-parens-after-r-keyword.
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.
The cause for the pylint warning was that we use Python 3 in Hadoop 3.x and we're still using Python 2 in Hadoop 2.x. Instead of importing the print_function
, could you please rewrite the print statement using Python 2 syntax? I feel this is a better approach since all the Python scripts in Hadoop 2.x is written in Python 2.
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.
@GauthamBanasandra sorry, I missed this comment.I will rewrite the print statement using Python 2 syntax.
@@ -0,0 +1,30 @@ | |||
#!/usr/bin/env bash |
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 file isn't being used anywhere. Please delete it.
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.
@GauthamBanasandra ok, I will remove this file.
(!) A patch to the testing environment has been detected. |
💔 -1 overall
This message was automatically generated. |
@GauthamBanasandra Do you have any problems for this PR ? |
@ZhendongBai I'm still waiting for you to resolve this - #3535 (comment) |
(!) A patch to the testing environment has been detected. |
💔 -1 overall
This message was automatically generated. |
@ZhendongBai your PR has failed for asflicense check. This was fixed in trunk. I've created a PR to backport this fix - #3583. Your PR looks good now. Once I merge my PR, please rebase yours and the asflicense failure should get resolved. We can then merge your PR. |
…from Dockerfile_centos_7. and we can use `docker build -t hadoop-build -f dev-support/docker/Dockerfile_centos_7 dev-support/docker` to replace `docker build -t hadoop-build dev-support/docker `(in the file named start-build-env.sh) to build 2.10.x with Docker only.
…thon3. for more compatibility, use `sys.stderr.write()` instead of `print`
@GauthamBanasandra thanks a lot,I will rebase on branch-2.10 and run a full docker build and maven build process, and ensure this PR is really ok after modifications by comments. |
2. because python-pip not found at centos-release-scl repo, we need to install epel-release 3. downgrade python3 to python2 4. use python2 syntax instead of python3 5. use pip2 to install `configparser==4.0.2 pylint==1.9.2 isort==4.3.21 python-dateutil==2.7.3` instead of `pylint==2.6.0 python-dateutil==2.8.1` for pylint2.x is not compatible with python2.7 and so on.
dd80caf
to
b25fc63
Compare
(!) A patch to the testing environment has been detected. |
💔 -1 overall
This message was automatically generated. |
… SC2046 Quote this to prevent word splitting.`
(!) A patch to the testing environment has been detected. |
💔 -1 overall
This message was automatically generated. |
@GauthamBanasandra I do following things recently as bellow:
please review again, thanks. |
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.
Thanks for the PR @ZhendongBai. LGTM.
LGTM. I'll merge this PR tomorrow if there are no further comments. |
Description of PR
Getting Hadoop to build on Centos 7 will greatly benefit the community. Here, we aim to provide a Dockerfile that builds out the image with all the dependencies needed to build Hadoop on Centos 7.
How was this patch tested?
Tested on mac x86_64, CI runs.
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?