Skip to content

Conversation

@gaborgsomogyi
Copy link
Contributor

@gaborgsomogyi gaborgsomogyi commented May 26, 2020

What changes were proposed in this pull request?

There is an unnecessary dependency for mssql-jdbc. In this PR I've removed it.

Why are the changes needed?

Unnecessary dependency.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass the Jenkins with the following configuration.

@gaborgsomogyi
Copy link
Contributor Author

cc @dongjoon-hyun

@dongjoon-hyun
Copy link
Member

Thank you for making a PR, but it's weird. This is an auto-generated file. Are you sure this is enough?

@SparkQA
Copy link

SparkQA commented May 26, 2020

Test build #123117 has finished for PR 28640 at commit 4b333b2.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member

You shall not pass. :)

@gaborgsomogyi gaborgsomogyi changed the title [SPARK-31821][BUILD]Remove mssql-jdbc dependencies [WIP][SPARK-31821][BUILD]Remove mssql-jdbc dependencies May 26, 2020
@HeartSaVioR
Copy link
Contributor

HeartSaVioR commented May 26, 2020

Isn't it weird it's only pulled from Hive 2.3? Does Hive 2.3 refer any of mssql?

@dongjoon-hyun
Copy link
Member

Ya. Right, @HeartSaVioR . This dependency also surprised me at @gaborgsomogyi 's PR. We need to remove this definitely.

@gaborgsomogyi
Copy link
Contributor Author

There is another dep file with hive 2.3 where it doesn't appear. What I can think of is hadoop 3.2...

@gaborgsomogyi
Copy link
Contributor Author

It's weird, coming from spark-yarn.

@gaborgsomogyi
Copy link
Contributor Author

...
[INFO] +- org.apache.hadoop:hadoop-yarn-server-web-proxy:jar:3.2.0:compile
[INFO] |  \- org.apache.hadoop:hadoop-yarn-server-common:jar:3.2.0:compile
[INFO] |     +- org.apache.hadoop:hadoop-yarn-registry:jar:3.2.0:compile
[INFO] |     |  \- commons-daemon:commons-daemon:jar:1.0.13:compile
[INFO] |     +- org.apache.geronimo.specs:geronimo-jcache_1.0_spec:jar:1.0-alpha-1:compile
[INFO] |     +- org.ehcache:ehcache:jar:3.3.1:compile
[INFO] |     \- com.microsoft.sqlserver:mssql-jdbc:jar:6.2.1.jre7:runtime
...

@gaborgsomogyi
Copy link
Contributor Author

gaborgsomogyi commented May 26, 2020

This has been added in YARN-3663 but not yet clear whether it's required on the client side.

@gaborgsomogyi
Copy link
Contributor Author

Based on the commit found here the dependency infiltrated on the client side is bug and not feature. We've excluded HikariCP-java7 as well added in the mentioned PR.

@gaborgsomogyi gaborgsomogyi changed the title [WIP][SPARK-31821][BUILD]Remove mssql-jdbc dependencies [SPARK-31821][BUILD]Remove mssql-jdbc dependencies May 26, 2020
<exclusion>
<groupId>com.microsoft.sqlserver</groupId>
<artifactId>mssql-jdbc</artifactId>
</exclusion>
Copy link
Member

Choose a reason for hiding this comment

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

Thank you so much!

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-31821][BUILD]Remove mssql-jdbc dependencies [SPARK-31821][BUILD] Remove mssql-jdbc dependencies from Hadoop 3.2 profile May 26, 2020
@dongjoon-hyun dongjoon-hyun changed the title [SPARK-31821][BUILD] Remove mssql-jdbc dependencies from Hadoop 3.2 profile [SPARK-31821][BUILD][test-hadoop3.2] Remove mssql-jdbc dependencies from Hadoop 3.2 profile May 26, 2020
@dongjoon-hyun
Copy link
Member

Retest this please.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-31821][BUILD][test-hadoop3.2] Remove mssql-jdbc dependencies from Hadoop 3.2 profile [SPARK-31821][BUILD][test-hadoop3.2][test-maven] Remove mssql-jdbc dependencies from Hadoop 3.2 profile May 26, 2020
@dongjoon-hyun
Copy link
Member

Retest this please.

@SparkQA
Copy link

SparkQA commented May 26, 2020

Test build #123123 has finished for PR 28640 at commit b6d2094.

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

@SparkQA
Copy link

SparkQA commented May 26, 2020

Test build #123128 has finished for PR 28640 at commit b6d2094.

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

@SparkQA
Copy link

SparkQA commented May 26, 2020

Test build #123129 has finished for PR 28640 at commit b6d2094.

  • This patch fails from timeout after a configured wait of 400m.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-31821][BUILD][test-hadoop3.2][test-maven] Remove mssql-jdbc dependencies from Hadoop 3.2 profile [SPARK-31821][BUILD] Remove mssql-jdbc dependencies from Hadoop 3.2 profile May 27, 2020
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a 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/3.0.

dongjoon-hyun pushed a commit that referenced this pull request May 27, 2020
…rofile

### What changes were proposed in this pull request?
There is an unnecessary dependency for `mssql-jdbc`. In this PR I've removed it.

### Why are the changes needed?
Unnecessary dependency.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?

Pass the Jenkins with the following configuration.
- [x] Pass the dependency test.
- [x] SBT with Hadoop-3.2 (#28640 (comment))
- [ ] Maven with Hadoop-3.2

Closes #28640 from gaborgsomogyi/SPARK-31821.

Authored-by: Gabor Somogyi <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 8f1d774)
Signed-off-by: Dongjoon Hyun <[email protected]>
@dongjoon-hyun
Copy link
Member

Thank you so much, @gaborgsomogyi .

@gaborgsomogyi
Copy link
Contributor Author

Just wanted to continue but already finished, thanks for the help @dongjoon-hyun !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants