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

[FLINK-28013] Shade all netty dependencies in sql jar #17

Merged
merged 1 commit into from
Aug 28, 2023

Conversation

whhe
Copy link
Member

@whhe whhe commented Jul 27, 2023

Shade all netty dependencies to resolve java.lang.NoClassDefFoundError: org/apache/flink/hbase/shaded/io/netty/channel/EventLoopGroup.

@boring-cyborg
Copy link

boring-cyborg bot commented Jul 27, 2023

Thanks for opening this pull request! Please check out our contributing guidelines. (https://flink.apache.org/contributing/how-to-contribute.html)

@whhe
Copy link
Member Author

whhe commented Jul 28, 2023

@MartijnVisser PTAL

Copy link
Contributor

@MartijnVisser MartijnVisser left a comment

Choose a reason for hiding this comment

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

@whhe The connector actually shouldn't rely on flink-shaded at all.

@whhe
Copy link
Member Author

whhe commented Jul 28, 2023

@whhe The connector actually shouldn't rely on flink-shaded at all.

It will not depend on 'flink-shaded', this modification is to add netty packages other than 'netty-all', such as 'netty-transport' here, to the sql jar.

@MartijnVisser
Copy link
Contributor

this modification is to add netty packages other than 'netty-all', such as 'netty-transport' here, to the sql jar.

Ah sorry, I misread. I'll try to have a look later today

@whhe
Copy link
Member Author

whhe commented Aug 1, 2023

Any progress? @MartijnVisser

@YesOrNo828
Copy link

I tested this patch locally which resolves this NoClassDefFoundError exception;
java.lang.NoClassDefFoundError: org/apache/flink/hbase/shaded/io/netty/channel/EventLoopGroup.

@whhe
Copy link
Member Author

whhe commented Aug 28, 2023

Hello again @MartijnVisser, please take a look on this pr if you are available. It wouldn't take a long time.

@MartijnVisser MartijnVisser merged commit 8b794a3 into apache:main Aug 28, 2023
4 checks passed
@boring-cyborg
Copy link

boring-cyborg bot commented Aug 28, 2023

Awesome work, congrats on your first merged pull request!

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

Successfully merging this pull request may close these issues.

3 participants