Skip to content

[WIP] [fix #9640] fix pulsar-client-admin-api dependency - #9842

Closed
freeznet wants to merge 3 commits into
apache:masterfrom
freeznet:freeznet/fix-admin-api-dependency
Closed

[WIP] [fix #9640] fix pulsar-client-admin-api dependency#9842
freeznet wants to merge 3 commits into
apache:masterfrom
freeznet:freeznet/fix-admin-api-dependency

Conversation

@freeznet

@freeznet freeznet commented Mar 9, 2021

Copy link
Copy Markdown
Contributor

Fixes #9640

Motivation

Module pulsar-client-admin-api has been introduced in #9246, but it pulling in many dependencies.  That is undesirable for an "API" module. As @sijie @merlimat and @jerrypeng suggests, this pr resolves the issue and make pulsar-client-admin-api has minimal transient dependences.

Modifications

  • pulsar-client-admin-api should only depend on pulsar-client-api
  • Move pojos from pulsar-common to pulsar-client-admin-api module
  • Use lombok to expose fields in pojos with autogenerated getters

Verifying this change

  • Make sure that the change passes the CI checks.
  • Check pulsar-client-admin-api with minimal transient dependences.
$ mvn -pl pulsar-client-admin-api dependency:tree -Dscope=runtime
[INFO] --- maven-dependency-plugin:3.1.2:tree (default-cli) @ pulsar-client-admin-api ---
[INFO] org.apache.pulsar:pulsar-client-admin-api:jar:2.8.0-SNAPSHOT
[INFO] \- org.apache.pulsar:pulsar-client-api:jar:2.8.0-SNAPSHOT:compile
[INFO] ------------------------------------------------------------------------

Changes need to confirm

  • in order to prevent the dependency of org.apache.pulsar.common.api.proto.CommandSubscribe.SubType, I have changed the type from SubType to String in pulsar-client-admin-api module. [DONE]
  • PulsarAdmin#getClientConfigData() returns ClientConfigurationData from org.apache.pulsar.client.impl.conf, which will cause unnecessary dependency to pulsar-client-original. [WIP]

<version>${project.parent.version}</version>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
<scope>provided</scope>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This API package shouldn't depend on anything more than pulsar-client-api.

Having the dependencies marked as "provided" doesn't really fix the issue because the implementation will still be using them. We should instead make sure that the POJOs are not using any other dependency.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yea, thanks for your comment. will update the pr once done.

@freeznet
freeznet force-pushed the freeznet/fix-admin-api-dependency branch from d40b445 to 636e6d1 Compare March 10, 2021 15:01
@jerrypeng

Copy link
Copy Markdown
Contributor

@freeznet what is the status of this work?

@freeznet

freeznet commented May 8, 2021

Copy link
Copy Markdown
Contributor Author

close this PR and addressed the issue with PR #10513.

@freeznet freeznet closed this May 8, 2021
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.

pulsar-client-admin-api has excessive transient dependencies

3 participants