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

[Bug] slf4j api class files included in the shaded client jar #129

Closed
nodece opened this issue Apr 8, 2024 · 8 comments
Closed

[Bug] slf4j api class files included in the shaded client jar #129

nodece opened this issue Apr 8, 2024 · 8 comments

Comments

@nodece
Copy link

nodece commented Apr 8, 2024

I'm migrating the slf4j to 2.0.12 from 1.7.32 on the Apache Pulsar, I noticed the oxia-client shaded includes the slf4j package, and the maven-shaded-plugin cannot exclude the slf4j package from the oxia-client shaded jar:

image

Could you remove the slf4j package from the shaded jar.

Apache Pulsar PR: apache/pulsar#22391

/cc @lhotari @merlimat

@lhotari
Copy link
Member

lhotari commented Apr 8, 2024

There's also another problem that the POM doesn't look right for the unshaded version: https://repo1.maven.org/maven2/io/streamnative/oxia/oxia-client/0.1.1/oxia-client-0.1.1.pom . It seems that the shaded library would need to be published as a different artifact to support having a separate POM for the shaded library and the unshaded library. At least that's how it is addressed in Pulsar pulsar-client (shaded) and pulsar-client-original (non-shaded).

@lhotari
Copy link
Member

lhotari commented Apr 8, 2024

In Pulsar, these settings are passed to the shade plugin:

              <createDependencyReducedPom>true</createDependencyReducedPom>
              <promoteTransitiveDependencies>true</promoteTransitiveDependencies>
              <minimizeJar>false</minimizeJar>

https://github.com/apache/pulsar/blob/57a616eaa79096af5b49db89c99cd39ccc94ec00/pulsar-client-shaded/pom.xml#L125-L127

However, this would require having a solution where the shaded and non-shaded libraries have a separate POM.

@lhotari
Copy link
Member

lhotari commented Apr 8, 2024

Another shading issue is that it seems to be shading the already shaded netty pulled in with grpc. This could be a problem in some cases since the native libraries aren't properly relocated.

@lhotari
Copy link
Member

lhotari commented Apr 8, 2024

@nodece please rename the issue report. Something like "DO NOT include the slf4j package to the shaded jar" isn't a great title for an issue report. A better one would be "[Bug] slf4j api class files included in the shaded client jar".

@nodece nodece changed the title DO NOT include the slf4j package to the shaded jar [Bug] slf4j api class files included in the shaded client jar Apr 8, 2024
@merlimat
Copy link
Collaborator

merlimat commented Apr 8, 2024

Good point, we can create a separate module to do the shading, instead of using the classifier.

Also, I think the only reason to use shaded jar in Pulsar was a conflict with gRPC. I was trying to downgrade gRPC here so that we can use same version as Pulsar and remove the need for shading.

@nodece
Copy link
Author

nodece commented Apr 9, 2024

For my issue, just to remove the sfl4j from the shaded jar.

Using the different modules to package the original and shaded jar is a good idea.

Also, I think the only reason to use shaded jar in Pulsar was a conflict with gRPC. I was trying to downgrade gRPC here so that we can use same version as Pulsar and remove the need for shading.

When using the original, the oxia needs to match the pulsar netty. Is it correct?

@merlimat
Copy link
Collaborator

merlimat commented Apr 9, 2024

When using the original, the oxia needs to match the pulsar netty. Is it correct?

Biggest problem is gRPC that has to match, across BK, Pulsar and now Oxia client

@nodece
Copy link
Author

nodece commented Apr 24, 2024

This issue has been fixed by oxia@0.1.6.

@nodece nodece closed this as completed Apr 24, 2024
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

No branches or pull requests

3 participants