-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-27402][INFRA][FOLLOW-UP] Exclude 'hive-thriftserver' in modules to test for hadoop3.2 for now #24644
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
| >>> x = [x.name for x in determine_modules_to_test([modules.sql])] | ||
| >>> x # doctest: +NORMALIZE_WHITESPACE | ||
| ... # doctest: +SKIP | ||
| ['sql', 'avro', 'hive', 'mllib', 'sql-kafka-0-10', 'examples', 'hive-thriftserver', |
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.
l manually tested with HADOOP_PROFILE=hadoop3.2 python -m doctest run-tests.py and python -m doctest run-tests.py
| hadoop_version = os.environ.get("HADOOP_PROFILE", "hadoop2.7") | ||
| if hadoop_version == "hadoop3.2": | ||
| print("[info] Skip unsupported module:", "hive-thriftserver") | ||
| all_modules = [m for m in all_modules if m.name != "hive-thriftserver"] |
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.
Similarily, I tested all_modules. This is just to move the codes to remove later in single place.
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 just does the same thing as before, right?
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.
yea I just moved.
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.
Got it. Thank you.
|
Thank you so much, @HyukjinKwon ! |
This reverts commit fd05eaf.
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
Outdated
Show resolved
Hide resolved
| else: | ||
| hadoop_version = os.environ.get("HADOOP_PROFILE", "hadoop2.7") | ||
| print("[info] Choosing supported modules with Hadoop profile", hadoop_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.
Since this is a partial revert of SPARK-27402, ping @wangyum .
| changed_modules = toposort_flatten( | ||
| {m: set(m.dependencies).intersection(modules_to_test) for m in modules_to_test}, sort=True) | ||
|
|
||
| # TODO: Skip hive-thriftserver module for hadoop-3.2. remove this once hadoop-3.2 support 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.
Could we revert modules.py to https://github.com/apache/spark/blob/3729efb4d0420700a396c79a83a1d5db25ac3bcb/dev/sparktestsupport/modules.py and just keep this change is enough?
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.
Just keeping this change isn't enough because build and some other checks use profiles from root's build_profile_flags.
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.
OK. Thank you @HyukjinKwon
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.
To be clear, this fix itself is a temp fix too in order to make the affected scope isolated and minimised. To fix it properly, we should maybe be able to fix it within Module at modules.py (and remove this entire module conditionally)
|
profiles look fine so far. I will switch to SBT's to make sure it doesn't affect our regular builds. |
|
retest this please |
|
retest this please |
|
okie profiles look good. |
|
Test build #105544 has finished for PR 24644 at commit
|
|
Test build #105545 has finished for PR 24644 at commit
|
|
retest this please |
|
Test build #105543 has finished for PR 24644 at commit
|
|
Test build #105542 has finished for PR 24644 at commit
|
|
@HyukjinKwon May be need add |
|
Test build #105541 has finished for PR 24644 at commit
|
|
Test build #105547 has finished for PR 24644 at commit
|
|
Do you mean regular PR build with hadoop 2.7 is affected? I am trying to see if this PR affects regular build with hadoop 2.7 |
|
retest this please |
|
hadoop-3.2 needs |
|
yea thanks for clarification. |
|
Test build #105557 has finished for PR 24644 at commit
|
|
Okie. I think it's ready if I am not mistaken. |
dongjoon-hyun
left a comment
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, LGTM. Merged to master.
|
Thanks, @dongjoon-hyun |
What changes were proposed in this pull request?
This PR excludes 'hive-thriftserver' in modules to test for hadoop3.2 for now as well
How was this patch tested?
Manually tested via
run-tests.py