[SPARK-6153] [SQL] promote guava dep for hive-thriftserver

[adrian-wang]

For package thriftserver, guava is used at runtime.

/cc @pwendell

[SparkQA]

Test build #28251 has started for PR 4884 at commit 44dda18.

• This patch merges cleanly.

[SparkQA]

Test build #28251 has finished for PR 4884 at commit 44dda18.

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

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28251/
Test PASSed.

[srowen]

I don't see any usages of Guava in these modules. Why would they need a compile time dependency?
What problem are you solving here?

[liancheng]

I have the same question as @srowen. Is it related to the binary incompatiblity issue Guava 15/17 introduced? (Probably no?)

[adrian-wang]

@srowen @liancheng You can reproduce my problem by

1. clone spark
2. import it into intellij by choose import from maven
3. run sparksqlclidriver in intellij.

About the usage of guava in module hive, check https://github.com/apache/spark/blob/master/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala#L23

The usage of guava in thriftserver is called by a dependency of module thriftserver. This should be ok, but the root pom specifically mark guava as scope provided. This would not lead to a real problem, but only affect what maven plugin of intellij does.

Thanks for your review!

[srowen]

There can't be a need to add a compile scope dependency on Guava since it is obviously not needed at compile time. That should be removed. I'm not sure that the SparkSQLCLIDriver is intended to be runnable from an IDE, but ideally it should. I also see it fails for not seeing Guava classes. At best, I imagine that both Hive modules could declare a runtime dependency on Guava. I think it would not disrupt the shading in the final assembly. Can we try that instead?

[adrian-wang]

I had a small talk with @liancheng , and Guava for module hive with level 'provided' works for him when trying to reproduce my issue, while it doesn't work for me.
For the hive-thriftserver module, both of us need to promote Guava to level 'Runtime'.

/cc @srowen , set guava to runtime level for hive module still doesn't work for me.

[adrian-wang]

BTW, I am using Intellij community version 14.0.3 on Ubuntu 12.04.

[liancheng]

I suspect this is more like a local dev environment configuration issue. Though @srowen' suggestion makes sense. While trying to run the CLI within IDEA, I didn't encounter compilation error, but did encounter runtime error (Guava classes not found).

[adrian-wang]

Yes, I cleaned idea cache and the compilation error is gone.

[SparkQA]

Test build #28280 has started for PR 4884 at commit 4600ae7.

• This patch merges cleanly.

[liancheng]

Now this LGTM. However, I believe @pwendell is cutting RC2, and this issue is IDEA specific, so I'm not going to merge this to branch-1.3 right now. Let's wait for Jenkins first.

[liancheng]

@adrian-wang Please also fix the PR description, thanks!

[liancheng]

Also, I think this PR is not about deploy?

[adrian-wang]

updated, thanks!

[SparkQA]

Test build #28280 has finished for PR 4884 at commit 4600ae7.

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

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28280/
Test PASSed.

[liancheng]

RC2 had already been cut, so merged into master and branch-1.3. Thanks for working on this!

[srowen]

Hm, I would have liked @vanzin to review this first. We handle Guava quite specially with the strange provided scope to make shading work out. Since we will need another RC for 1.3, and this isn't a critical change, I am not sure it should have gone in to branch-1.3. We need to figure out where this affects the shading or not for sure.

[liancheng]

@srowen I have been fighting with compatibilities issues related to Guava 15.0 and 17.0 for a few days recently (#4872 & #4890). To the best of my knowledge, this change should be safe. But I have to admit this is a pretty nasty and complex issue that worth double or even triple checking. I'm going to revert this on branch-1.3 for safe. (My first reverting operation, crossing fingers...)

@vanzin It would be great if you can have a close look at this issue. Thanks!

[liancheng]

Reverted this on branch-1.3.

[vanzin]

I really can't comment much just based on the change... when conflicts like this happen, it's always useful to have the output of "mvn dependency:tree -Dverbose" for the affected module. With that we can figure out exactly what version with what scope is coming from where, and we have better info to come up with the appropriate fix.

[liancheng]

Thanks @vanzin, I'll double check with mvn dependency:tree -Dverbose to see whether this change introduces any negative impacts.

[liancheng]

@vanzin @srowen Double checked Guava verison and scope in the dependency tree. It should be fine.