Skip to content
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

[SPARK-29991][INFRA] Support test-hive1.2 in PR Builder #26695

Closed
wants to merge 1 commit into from
Closed

[SPARK-29991][INFRA] Support test-hive1.2 in PR Builder #26695

wants to merge 1 commit into from

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Nov 27, 2019

What changes were proposed in this pull request?

Currently, Apache Spark PR Builder using hive-1.2 for hadoop-2.7 and hive-2.3 for hadoop-3.2. This PR aims to support [test-hive1.2] in PR Builder in order to cut the correlation between hive-1.2/2.3 to hadoop-2.7/3.2. After this PR, the PR Builder will use hive-2.3 by default for all profiles (if there is no test-hive1.2.)

Why are the changes needed?

This new tag allows us more flexibility.

Does this PR introduce any user-facing change?

No. (This is a dev-only change.)

How was this patch tested?

Check the Jenkins triggers in this PR.

BEFORE

========================================================================
Building Spark
========================================================================
[info] Building Spark using SBT with these arguments:  -Phadoop-2.7 -Phive-1.2 -Pyarn -Pkubernetes -Phive -Phadoop-cloud -Pspark-ganglia-lgpl -Phive-thriftserver -Pkinesis-asl -Pmesos test:package streaming-kinesis-asl-assembly/assembly

AFTER

  1. Title: [SPARK-29991][INFRA][test-hive1.2] Support test-hive1.2 in PR Builder
========================================================================
Building Spark
========================================================================
[info] Building Spark using SBT with these arguments:  -Phadoop-2.7 -Phive-1.2 -Pkinesis-asl -Phadoop-cloud -Pyarn -Phive -Pmesos -Pspark-ganglia-lgpl -Pkubernetes -Phive-thriftserver test:package streaming-kinesis-asl-assembly/assembly
  1. Title: [SPARK-29991][INFRA] Support test hive1.2 in PR Builder
  • Note that I removed the hyphen intentionally from test-hive1.2.
========================================================================
Building Spark
========================================================================
[info] Building Spark using SBT with these arguments:  -Phadoop-2.7 -Phive-thriftserver -Pkubernetes -Pspark-ganglia-lgpl -Phadoop-cloud -Phive -Pmesos -Pyarn -Pkinesis-asl test:package streaming-kinesis-asl-assembly/assembly

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-29991][INFRA][test-hive1.2] Support [test-hive1.2] in PR Builder [WIP][SPARK-29991][INFRA][test-hive1.2] Support [test-hive1.2] in PR Builder Nov 27, 2019
@dongjoon-hyun dongjoon-hyun changed the title [WIP][SPARK-29991][INFRA][test-hive1.2] Support [test-hive1.2] in PR Builder [SPARK-29991][INFRA][test-hive1.2] Support [test-hive1.2] in PR Builder Nov 27, 2019
@dongjoon-hyun dongjoon-hyun changed the title [SPARK-29991][INFRA][test-hive1.2] Support [test-hive1.2] in PR Builder [SPARK-29991][INFRA][test-hive1.2] Support test-hive1.2 in PR Builder Nov 27, 2019
@SparkQA

This comment has been minimized.

@SparkQA

This comment has been minimized.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-29991][INFRA][test-hive1.2] Support test-hive1.2 in PR Builder [SPARK-29991][INFRA] Support test hive1.2 in PR Builder Nov 27, 2019
@dongjoon-hyun
Copy link
Member Author

Retest this please.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-29991][INFRA] Support test hive1.2 in PR Builder [SPARK-29991][INFRA] Support test-hive1.2 in PR Builder Nov 27, 2019
@SparkQA

This comment has been minimized.

@SparkQA
Copy link

SparkQA commented Nov 28, 2019

Test build #114550 has finished for PR 26695 at commit 264e64c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Nov 28, 2019

Test build #114551 has finished for PR 26695 at commit 264e64c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member Author

Hi, @gatorsmile , @srowen , @HyukjinKwon .
Could you review this?

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If nothing else, it's a step away from Hive 1 as any kind of default

@dongjoon-hyun
Copy link
Member Author

Thank you for review and approval, @srowen . Yes. Right.
After this PR, the remaining thing is SPARK-29988 to protect hive-1.2 as a side profile.

@dongjoon-hyun
Copy link
Member Author

Merged to master.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-29991 branch November 28, 2019 05:15
}

if hadoop_version in sbt_maven_hadoop_profiles:
return sbt_maven_hadoop_profiles[hadoop_version]
if ("ghprbPullTitle" in os.environ and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do it consistently with what we handle Hadoop versions? (e.g., 8ab1306#diff-3c9f4fccf7d30ce2e8fa86db2ad1fdad)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and .. seems like we should change c98e5eb#diff-180360612c6b8c4ed830919bbb4dd459R56 too if this PR also targets to change default Hive profile to 2.3.

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Nov 29, 2019

Seems this PR caused the test failure - https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/114582/testReport/

org.apache.spark.sql.hive.thriftserver.ui.ThriftServerPageSuite.thriftserver page should load 
org.apache.spark.sql.hive.thriftserver.ui.ThriftServerPageSuite.thriftserver session page should load successfully

It looks like the tests against Hive 2.3 didn't run because the PR title constantly has test-hive1.2 .. :-) ..

After reverting this, seems the tests were recovered (#26706)

@HyukjinKwon
Copy link
Member

Since this change blocks other PRs, let me revert this one for now.

attilapiros pushed a commit to attilapiros/spark that referenced this pull request Dec 6, 2019
### What changes were proposed in this pull request?

Currently, Apache Spark PR Builder using `hive-1.2` for `hadoop-2.7` and `hive-2.3` for `hadoop-3.2`. This PR aims to support `[test-hive1.2]` in PR Builder in order to cut the correlation between `hive-1.2/2.3` to `hadoop-2.7/3.2`. After this PR, the PR Builder will use `hive-2.3` by default for all profiles (if there is no `test-hive1.2`.)

### Why are the changes needed?

This new tag allows us more flexibility.

### Does this PR introduce any user-facing change?

No. (This is a dev-only change.)

### How was this patch tested?

Check the Jenkins triggers in this PR.

**BEFORE**
```
========================================================================
Building Spark
========================================================================
[info] Building Spark using SBT with these arguments:  -Phadoop-2.7 -Phive-1.2 -Pyarn -Pkubernetes -Phive -Phadoop-cloud -Pspark-ganglia-lgpl -Phive-thriftserver -Pkinesis-asl -Pmesos test:package streaming-kinesis-asl-assembly/assembly
```

**AFTER**
1. Title: [[SPARK-29991][INFRA][test-hive1.2] Support `test-hive1.2` in PR Builder](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/114550/testReport)
```
========================================================================
Building Spark
========================================================================
[info] Building Spark using SBT with these arguments:  -Phadoop-2.7 -Phive-1.2 -Pkinesis-asl -Phadoop-cloud -Pyarn -Phive -Pmesos -Pspark-ganglia-lgpl -Pkubernetes -Phive-thriftserver test:package streaming-kinesis-asl-assembly/assembly
```

2. Title: [[SPARK-29991][INFRA] Support `test hive1.2` in PR Builder](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/114551/testReport)
- Note that I removed the hyphen intentionally from `test-hive1.2`.
```
========================================================================
Building Spark
========================================================================
[info] Building Spark using SBT with these arguments:  -Phadoop-2.7 -Phive-thriftserver -Pkubernetes -Pspark-ganglia-lgpl -Phadoop-cloud -Phive -Pmesos -Pyarn -Pkinesis-asl test:package streaming-kinesis-asl-assembly/assembly
```

Closes apache#26695 from dongjoon-hyun/SPARK-29991.

Authored-by: Dongjoon Hyun <dhyun@apple.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants