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

Build: Exclude zstd-jni from iceberg-spark3-runtime #3058

Merged
merged 1 commit into from
Sep 9, 2021

Conversation

pan3793
Copy link
Member

@pan3793 pan3793 commented Aug 31, 2021

spark-core already has the dependency zstd-jni, it should be exclude from iceberg-spark3-runtime.

@pan3793 pan3793 changed the title Exclude zstd-jni from iceberg-spark3-runtime Build: Exclude zstd-jni from iceberg-spark3-runtime Aug 31, 2021
@github-actions github-actions bot added the build label Aug 31, 2021
Copy link
Contributor

@kbendick kbendick left a comment

Choose a reason for hiding this comment

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

+1. We have tested this internally and it is needed as this can't be shaded (it's a dependency that comes from parquet 1.12 by means of spark).

@pan3793
Copy link
Member Author

pan3793 commented Sep 2, 2021

cc @RussellSpitzer, please take a look if you have time.

@RussellSpitzer
Copy link
Member

Confirming what @kbendick said we made this change internally a while back. If everyone else is ok with it I think we should remove the library but this is the kind of change I think we need more folks to check out before we merge just incase we are missing someone's prod usage. @rdblue + @jackye1995 + @openinx

@rdblue
Copy link
Contributor

rdblue commented Sep 2, 2021

Sounds good to me.

@pan3793
Copy link
Member Author

pan3793 commented Sep 8, 2021

@jackye1995 would u pls take a look as well?

@rdblue rdblue merged commit 6d80968 into apache:master Sep 9, 2021
@rdblue
Copy link
Contributor

rdblue commented Sep 9, 2021

Thanks, @pan3793! I went ahead and merged this since it's been a few days without comment.

@kbendick
Copy link
Contributor

I added this to 0.13.0 milestone as it appears it didn't ship with 0.12.1.

I'm not sure if this merits a patch release to 0.12.2... I'd say since we're in progress towards 0.13.0 that it would be easier to hold off.

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