Skip to content
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

[WIP] Shade trino-parser dependency of coral-trino #205 #220

Closed
wants to merge 2 commits into from

Conversation

shenodaguirguis
Copy link

shading trino-parser dependeny [https://github.com//issues/205]

@@ -5,7 +5,8 @@ def modules = [
'coral-spark',
'coral-schema',
'coral-spark-plan',
'coral-trino'
'coral-trino',
'shading'
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we be more specific on what it shades, or are we foresee other shading components ?

@@ -0,0 +1,22 @@
import com.github.jengelman.gradle.plugins.shadow.tasks.ConfigureShadowRelocation
Copy link
Member

@rzhang10 rzhang10 Jan 13, 2022

Choose a reason for hiding this comment

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

I'm not sure if it's a good idea to make the shaded librarie as a separate module, and let the previous module depend on it (rather than just shade within the previous module directly)

But if we go this route, we need to also publish this shaded module (ref: https://imperceptiblethoughts.com/shadow/publishing/#publishing-with-maven-publish-plugin) , so that the downstream can continue to consume coral as a library.

@wmoustafa
Copy link
Contributor

wmoustafa commented Jan 25, 2022

This caused release failures due to the following error:

Monday, January 24, 2022 16:58:46 PST (GMT-0800)
typeId	signature-staging
failureMessage	Missing Signature: '/com/linkedin/coral/coral-trino-parser/2.0.46/coral-trino-parser-2.0.46-all.jar.asc' does not exist for 'coral-trino-parser-2.0.46-all.jar'.
failureMessage	Missing Signature: '/com/linkedin/coral/coral-trino-parser/2.0.46/coral-trino-parser-2.0.46.pom.asc' does not exist for 'coral-trino-parser-2.0.46.pom'.

We should not publish the trino-parser shaded jar and only use the classes internally I think.

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.

4 participants