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

Include pulsar-client-admin-api in the shaded version of pulsar-client-admin #9689

Merged
merged 1 commit into from
Feb 24, 2021

Conversation

jerrypeng
Copy link
Contributor

In the following PR:

#9246

The API of the pulsar-client-admin was separated into another module. However, the api module was not added to be included in the shaded version of pulsar-client-admin. This creates dependency issues such as

java.lang.NoSuchMethodError: org.apache.pulsar.client.admin.PulsarAdminException.<init>(Lorg/apache/pulsar/shade/javax/ws/rs/ClientErrorException;)V

@jerrypeng jerrypeng added the type/bug The PR fixed a bug or issue reported a bug label Feb 23, 2021
@jerrypeng jerrypeng added this to the 2.8.0 milestone Feb 23, 2021
@jerrypeng jerrypeng self-assigned this Feb 23, 2021
@@ -66,6 +66,7 @@
<include>org.apache.pulsar:pulsar-client-original</include>
<include>org.apache.pulsar:pulsar-client-api</include>
<include>org.apache.pulsar:pulsar-client-admin-original</include>
<include>org.apache.pulsar:pulsar-client-admin-api</include>
Copy link
Contributor

Choose a reason for hiding this comment

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

Typically, the API module should be left out of the shaded jar, instead being a transitive dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@merlimat pulsar-client-api is included as well. The problem is with shading. If pulsar-client-admin-api is not included in the shaded JAR and is just pull in as a transitive dependency errors like

java.lang.NoSuchMethodError: org.apache.pulsar.client.admin.PulsarAdminException.<init>(Lorg/apache/pulsar/shade/javax/ws/rs/ClientErrorException;)V

will occur because the impl will be using the shaded version classes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @jerrypeng
we already have org.apache.pulsar:pulsar-client-apiis it an error ?

@sijie
Copy link
Member

sijie commented Feb 24, 2021

@freeznet Can you review this? I think this is related to one task you are working on.

@@ -66,6 +66,7 @@
<include>org.apache.pulsar:pulsar-client-original</include>
<include>org.apache.pulsar:pulsar-client-api</include>
<include>org.apache.pulsar:pulsar-client-admin-original</include>
<include>org.apache.pulsar:pulsar-client-admin-api</include>
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @jerrypeng
we already have org.apache.pulsar:pulsar-client-apiis it an error ?

@jerrypeng jerrypeng merged commit e65deaf into apache:master Feb 24, 2021
merlimat pushed a commit to merlimat/pulsar that referenced this pull request Apr 6, 2021
…t-admin (apache#9689)

Co-authored-by: Jerry Peng <jerryp@splunk.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants