You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/03/09 03:53:37 UTC
[GitHub] [pulsar] freeznet opened a new pull request #9842: [WIP] [fix #9640] fix pulsar-client-admin-api dependency
freeznet opened a new pull request #9842:
URL: https://github.com/apache/pulsar/pull/9842
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] ------------------------------------------------------------------------
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on a change in pull request #9842: [WIP] [fix #9640] fix pulsar-client-admin-api dependency
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #9842:
URL: https://github.com/apache/pulsar/pull/9842#discussion_r590763057
##########
File path: pulsar-client-admin-api/pom.xml
##########
@@ -35,20 +35,54 @@
<dependencies>
<dependency>
<groupId>${project.groupId}</groupId>
- <artifactId>pulsar-common</artifactId>
+ <artifactId>pulsar-client-api</artifactId>
<version>${project.parent.version}</version>
</dependency>
<dependency>
- <groupId>${project.groupId}</groupId>
- <artifactId>pulsar-client-original</artifactId>
- <version>${project.parent.version}</version>
+ <groupId>com.google.code.gson</groupId>
+ <artifactId>gson</artifactId>
+ <scope>provided</scope>
Review comment:
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet commented on a change in pull request #9842: [WIP] [fix #9640] fix pulsar-client-admin-api dependency
Posted by GitBox <gi...@apache.org>.
freeznet commented on a change in pull request #9842:
URL: https://github.com/apache/pulsar/pull/9842#discussion_r590936368
##########
File path: pulsar-client-admin-api/pom.xml
##########
@@ -35,20 +35,54 @@
<dependencies>
<dependency>
<groupId>${project.groupId}</groupId>
- <artifactId>pulsar-common</artifactId>
+ <artifactId>pulsar-client-api</artifactId>
<version>${project.parent.version}</version>
</dependency>
<dependency>
- <groupId>${project.groupId}</groupId>
- <artifactId>pulsar-client-original</artifactId>
- <version>${project.parent.version}</version>
+ <groupId>com.google.code.gson</groupId>
+ <artifactId>gson</artifactId>
+ <scope>provided</scope>
Review comment:
yea, thanks for your comment. will update the pr once done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet closed pull request #9842: [WIP] [fix #9640] fix pulsar-client-admin-api dependency
Posted by GitBox <gi...@apache.org>.
freeznet closed pull request #9842:
URL: https://github.com/apache/pulsar/pull/9842
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet commented on pull request #9842: [WIP] [fix #9640] fix pulsar-client-admin-api dependency
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #9842:
URL: https://github.com/apache/pulsar/pull/9842#issuecomment-835209379
close this PR and addressed the issue with PR #10513.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jerrypeng commented on pull request #9842: [WIP] [fix #9640] fix pulsar-client-admin-api dependency
Posted by GitBox <gi...@apache.org>.
jerrypeng commented on pull request #9842:
URL: https://github.com/apache/pulsar/pull/9842#issuecomment-812175079
@freeznet what is the status of this work?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org