Skip to content

Conversation

@tedyu
Copy link
Contributor

@tedyu tedyu commented Jan 30, 2016

https://groups.google.com/forum/#!topic/protobuf/wAqvtPLBsE8

PB2 and PB3 are wire compatible, but, protobuf-java is not compatible so dependency will be a problem.
Shading protobuf-java would provide better experience for downstream projects.

This PR shades com.google.protobuf:protobuf-java as org.spark-project.protobuf

@tedyu
Copy link
Contributor Author

tedyu commented Jan 30, 2016

Would like some feedback before creating a JIRA.

Thanks

@SparkQA
Copy link

SparkQA commented Jan 30, 2016

Test build #50447 has finished for PR 10995 at commit 21cbc45.

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

@zsxwing
Copy link
Member

zsxwing commented Feb 1, 2016

Since Hadoop doesn't shade protobuf, I think this won't fix the issue in the PR description. Right?

@tedyu
Copy link
Contributor Author

tedyu commented Feb 1, 2016

This would still benefit Spark standalone and Spark on Mesos, right ?

For Spark on YARN, status quo is maintained.

@zsxwing
Copy link
Member

zsxwing commented Feb 1, 2016

@tedyu could you create a JIRA and add it to the title? This change is worth to have a JIRA to track.

Ping @srowen @JoshRosen to take a look.

@tedyu tedyu changed the title [test-maven] Shade protobuf-java [SPARK-13120] [test-maven] Shade protobuf-java Feb 1, 2016
@JoshRosen
Copy link
Contributor

Can you give this a descriptive PR description rather than a link to some mailing list thread?

@tedyu
Copy link
Contributor Author

tedyu commented May 6, 2016

@zsxwing @JoshRosen @srowen
Mind taking another look ?

Thanks

@vanzin
Copy link
Contributor

vanzin commented Aug 4, 2016

@zsxwing is right. Because Hadoop libraries don't shade protobuf, this will not help. You'd have to create custom Hadoop jars referencing the shaded protobuf classes, and that's a much bigger change.

(Both standalone and mesos still need hadoop jars, btw.)

srowen added a commit to srowen/spark that referenced this pull request Aug 27, 2016
@asfgit asfgit closed this in 1a48c00 Aug 29, 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.

5 participants