Skip to content

[SPARK-4261][BUILD][SQL] Make right version info for beeline #3124

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

Closed
wants to merge 5 commits into from

Conversation

scwf
Copy link
Contributor

@scwf scwf commented Nov 6, 2014

More detail discuss link to #3103
Build spark with maven, then use beeline to connect jdbc/odbc server, the output will be

JackydeMacBook-Pro:spark1 jackylee$ bin/beeline 
Spark assembly has been built with Hive, including Datanucleus jars on classpath
Beeline version ??? by Apache Hive

This PR make right version info for beeline by excluding hive-beeline from the assembly jar, and deliver it as a separate jar to "lib_managed/jars", a WIP tag here is because i have not tested this locally.

@scwf
Copy link
Contributor Author

scwf commented Nov 6, 2014

More detail discuss link to #3103

@SparkQA
Copy link

SparkQA commented Nov 6, 2014

Test build #22969 has started for PR 3124 at commit c19cd28.

  • This patch merges cleanly.

@scwf scwf changed the title [WIP][SPARK-4261] Make right version info for beeline [WIP][SPARK-4261][BUILD][SQL] Make right version info for beeline Nov 6, 2014
@SparkQA
Copy link

SparkQA commented Nov 6, 2014

Test build #22969 has finished for PR 3124 at commit c19cd28.

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

@AmplabJenkins
Copy link

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

@@ -95,6 +95,30 @@
<skip>true</skip>
</configuration>
</plugin>
<!-- Deploy datanucleus jars to the spark/lib_managed/jars directory -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix this

@scwf scwf changed the title [WIP][SPARK-4261][BUILD][SQL] Make right version info for beeline [SPARK-4261][BUILD][SQL] Make right version info for beeline Nov 6, 2014
@SparkQA
Copy link

SparkQA commented Nov 6, 2014

Test build #22989 has started for PR 3124 at commit 8011701.

  • This patch merges cleanly.

@scwf
Copy link
Contributor Author

scwf commented Nov 6, 2014

Updated and locally test ok. With this PR the output will be

JackydeMacBook-Pro:spark jackylee$ bin/beeline 
Spark assembly has been built with Hive, including Datanucleus jars on classpath
Beeline version 0.13.1a by Apache Hive

@scwf scwf changed the title [SPARK-4261][BUILD][SQL] Make right version info for beeline [SPARK-4261][BUILD][SQL] Make right version info for beeline when building with maven Nov 6, 2014
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<version>2.4</version>
Copy link
Member

Choose a reason for hiding this comment

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

Nit: you should just inherit version config from the parent pom

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems there is no maven-dependency-plugin version in parent Pom? @srowen

Copy link
Member

Choose a reason for hiding this comment

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

Oh, hm. I suggest adding it in <pluginManagement> -- just with the version. Also, use 2.9, not 2.4.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

@SparkQA
Copy link

SparkQA commented Nov 6, 2014

Test build #22989 has finished for PR 3124 at commit 8011701.

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

@AmplabJenkins
Copy link

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

@scwf
Copy link
Contributor Author

scwf commented Nov 6, 2014

Updated.

@SparkQA
Copy link

SparkQA commented Nov 6, 2014

Test build #22995 has started for PR 3124 at commit 4230322.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 6, 2014

Test build #22995 has finished for PR 3124 at commit 4230322.

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

@AmplabJenkins
Copy link

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

@scwf
Copy link
Contributor Author

scwf commented Nov 6, 2014

@pwendell, can you take a look at this?

@SparkQA
Copy link

SparkQA commented Nov 7, 2014

Test build #23035 has started for PR 3124 at commit 204bab1.

  • This patch merges cleanly.

@scwf scwf changed the title [SPARK-4261][BUILD][SQL] Make right version info for beeline when building with maven [SPARK-4261][BUILD][SQL] Make right version info for beeline Nov 7, 2014
@SparkQA
Copy link

SparkQA commented Nov 7, 2014

Test build #23035 has finished for PR 3124 at commit 204bab1.

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

@AmplabJenkins
Copy link

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

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@scwf
Copy link
Contributor Author

scwf commented Nov 8, 2014

cc @pwendell @marmbrus

@marmbrus
Copy link
Contributor

@pwendell any thoughts on the build changes here?

@scwf
Copy link
Contributor Author

scwf commented Nov 18, 2014

ping @pwendell

@scwf
Copy link
Contributor Author

scwf commented Nov 26, 2014

ping

1 similar comment
@scwf
Copy link
Contributor Author

scwf commented Dec 3, 2014

ping

@pwendell
Copy link
Contributor

pwendell commented Dec 3, 2014

Is Hive reading the version from a manifest somewhere? It's sort of unfortunate to have to increase the number of special case jars for a cosmetic change like this. Can we instead just add the relevant manifest file to our assembly?

@scwf
Copy link
Contributor Author

scwf commented Dec 3, 2014

Hi @pwendell, Beeline relies on the Implementation-Version field in the manifest to inspect version info. And my other PR fixed the manifest issue(#3103) of maven building, after that PR the manifest of assembly jar is as follows:

Manifest-Version: 1.0
Implementation-Vendor: org.apache.spark
Implementation-Title: spark-assembly
Implementation-Version: 1.2.0-SNAPSHOT
Implementation-Vendor-Id: org.apache.spark
Specification-Vendor: org.apache.spark
Specification-Title: spark-assembly
Specification-Version: 1.2.0-SNAPSHOT

which is same with manifest of SBT assembly jar.

The problem of using assembly jar is that Hive beeline version will be 1.2.0-SNAPSHOT since it read the manifest file of assembly jar. But we need the correct version of hive beeline, which should be 0.13.1a, so here i exclud hive-beeline from the assembly jar.

We can not read the right version if using assembly jar

@SparkQA
Copy link

SparkQA commented Dec 12, 2014

Test build #24409 has started for PR 3124 at commit 45ddd5b.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Dec 12, 2014

Test build #24409 has finished for PR 3124 at commit 45ddd5b.

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

@AmplabJenkins
Copy link

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

@scwf
Copy link
Contributor Author

scwf commented Dec 30, 2014

Ping

1 similar comment
@scwf
Copy link
Contributor Author

scwf commented Jan 8, 2015

Ping

@srowen
Copy link
Member

srowen commented Feb 10, 2015

I also disagree with this change, as it seems to be saying we should declare Spark's version in the Manifest to be incorrect, and match Beeline's, just to fix a cosmetic issue.

@scwf
Copy link
Contributor Author

scwf commented Feb 10, 2015

ok, I am closing this since this issue do not affect functional and as @srowen said it is a "cosmetic issue"

@scwf scwf closed this Feb 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants