Skip to content

Conversation

@yhuai
Copy link
Contributor

@yhuai yhuai commented Apr 21, 2016

What changes were proposed in this pull request?

This PR creates a compatibility module in sql (called hive-1-x-compatibility), which will host HiveContext in Spark 2.0 (moving HiveContext to here will be done separately). This module is not included in assembly because only users who still want to access HiveContext need it.

How was this patch tested?

I manually tested sbt/sbt -Phive package and mvn -Phive package -DskipTests.

Copy link
Contributor

Choose a reason for hiding this comment

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

add an extra line

@rxin
Copy link
Contributor

rxin commented Apr 21, 2016

I'd name this something like spark-1.x-compatibility (is dot allowed in the name?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like we should move sketch and compatibility out from this list once we release 2.0, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. The same goes for sketch and mllibLocal. Can you file a blocker JIRA for 2.0.1 / 2.1 to make sure that we audit this post-release?

@rxin
Copy link
Contributor

rxin commented Apr 21, 2016

also this should probably go into sql?

@SparkQA
Copy link

SparkQA commented Apr 21, 2016

Test build #56572 has finished for PR 12580 at commit 30e541d.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to pre-emptively add hive to this list.

@srowen
Copy link
Member

srowen commented Apr 21, 2016

Broadly, why is this a new and generic module? it seems quite Hive specific. I get that other fairly different compatibility shims could go here later, but those would properly be different modules right?

@yhuai
Copy link
Contributor Author

yhuai commented Apr 21, 2016

@srowen I am moving this into sql.

@SparkQA
Copy link

SparkQA commented Apr 21, 2016

Test build #56578 has started for PR 12580 at commit 7777ffb.

@yhuai
Copy link
Contributor Author

yhuai commented Apr 21, 2016

@JoshRosen https://issues.apache.org/jira/browse/SPARK-14818 is the jira for updating mima exclusions.

@yhuai
Copy link
Contributor Author

yhuai commented Apr 21, 2016

@srowen @rxin @JoshRosen How's this version?

@SparkQA
Copy link

SparkQA commented Apr 21, 2016

Test build #56580 has finished for PR 12580 at commit 92331f3.

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

<configuration>
<compilerArgs combine.children="append">
<!-- This option is needed to suppress warnings from sun.misc.Unsafe usage -->
<arg>-XDignore.symbol.file</arg>
Copy link
Contributor

Choose a reason for hiding this comment

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

Supressing warnings for sun.misc.unsafe sounds good, but do we need it in the hive compatibility shims project (this does suppress other warnings that we might want to see if we aren't planning on using sun.misc.unsafe here)?

@SparkQA
Copy link

SparkQA commented Apr 21, 2016

Test build #56604 has finished for PR 12580 at commit 3514b3c.

  • This patch fails executing the dev/run-tests script.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 21, 2016

Test build #56606 has started for PR 12580 at commit 166b34a.

@SparkQA
Copy link

SparkQA commented Apr 22, 2016

Test build #56608 has finished for PR 12580 at commit 8d54be3.

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

@JoshRosen
Copy link
Contributor

LGTM. We can always change the name later.

@asfgit asfgit closed this in 7dde1da Apr 23, 2016
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