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/05/25 12:02:21 UTC
[GitHub] [pulsar] freeznet opened a new pull request #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
freeznet opened a new pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697
Fixes #9640
Part 3
### Motivation
Module `pulsar-client-admin-api` has been introduced in #9246, but it pulling in many dependencies.
This PR removes `pulsar-common` from the dependency, and resolves the issues.
With this PR, some temp dependencies like `gson` [#10513], `swagger-annotations`, `slf4j-api`, and `commons-lang3` has been added. Later I will remove `gson` and `commons-lang3`, but I am not very sure about `swagger-annotations` and `slf4j-api`.
### Modifications
remove `pulsar-common` from `pulsar-client-admin-api`.
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
--
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] jiazhai commented on pull request #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#issuecomment-851776266
@eolivelli Would you please help review this PR again?
--
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 pull request #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
merlimat commented on pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#issuecomment-852284647
I cannot fix the conflicts here because I cannot push the branch. I'll push a new PR with this changes.
--
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 #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#issuecomment-851107233
/pulsarbot run-failure-checks
--
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 #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#issuecomment-851980218
@eolivelli thanks for your comments again.
> This makes me think that even if we are now allowing to use pure interfaces in order to define the API, this API is not usable without the "Impl" classes.
Interfaces are intended to define the APIs and specifications, which of cause not usable without the "impl" classes. So I don't think this is a problem here.
> So, what is the benefit of splitting the API classes from Impl ?
The main purpose of this PR is to solve #9640, and I am not splitting all the API classes into interface and impl class here, only those classes that will bring external dependencies into `pulsar-client-admin-api`. @lhotari has made a very clear description in #9640, and the discussion between @lhotari and @jerrypeng in #9638 is also very helpful to get to the context.
> will it be allowed to run something like:
```
class MyClusterData implements ClusterData {
.....
}
admin.clusters().createCluster("test", new MyClusterData());
```
Actually, I do not get this point very clear, it would be great if you could give me more background about how you came up with this idea?
> Probably we have to think more about this big breaking change, especially now that we are close to releasing 2.8.0.
You are right, so welcome to point it out if you have any specified concerns about this breaking change. You can also check [here](https://docs.google.com/document/d/19dXkVXeU2q_nHmkG8zURjKnYlvD96TbKf5KjYyASsOE/edit#bookmark=id.mwryw95kuerm) for some more details about the discussion on Pulsar Community Meeting.
--
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] sijie commented on pull request #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#issuecomment-852221913
My take here:
We are in a situation that we can’t roll back any of this change *easily. This change involves many PRs from many modules including client, client-admin, and functions. The earliest PR related to this change is probably a couple of months ago. So we need this change to land for 2.8.0
It is already a breaking change that we need to accept for pulsar-client-admin. It is hard to get to a very clean and ideal state in one release. I see what the PR proposed is the *best middle-ground situation that we can get between the 2.8.0 release and subsequent releases.
To unblock the 2.8.0 release, I think we can add @Evolving annotation to the pulsar client admin API to indicate there will be subsequent changes.
--
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 edited a comment on pull request #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
freeznet edited a comment on pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#issuecomment-851980218
@eolivelli thanks for your comments again.
> This makes me think that even if we are now allowing to use pure interfaces in order to define the API, this API is not usable without the "Impl" classes.
Interfaces are intended to define the APIs and specifications, which of cause not usable without the "impl" classes. So I don't think this is a problem here.
> So, what is the benefit of splitting the API classes from Impl ?
The main purpose of this PR is to solve #9640, and I am not splitting all the API classes into interface and impl class here, only those classes that will bring external dependencies into `pulsar-client-admin-api`. @lhotari has made a very clear description in #9640, and the discussion between @lhotari and @jerrypeng in #9638 is also very helpful to get to the context.
> will it be allowed to run something like:
```
class MyClusterData implements ClusterData {
.....
}
admin.clusters().createCluster("test", new MyClusterData());
```
Actually, I do not get this point very clear, it would be great if you could give me more background about how you came up with this idea?
> Probably we have to think more about this big breaking change, especially now that we are close to releasing 2.8.0.
You are right, so welcome to point it out if you have any other specified concerns about this breaking change. You can also check [here](https://docs.google.com/document/d/19dXkVXeU2q_nHmkG8zURjKnYlvD96TbKf5KjYyASsOE/edit#bookmark=id.mwryw95kuerm) for some more details about the discussion on Pulsar Community Meeting.
--
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] eolivelli commented on a change in pull request #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#discussion_r638726107
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/NonPersistentPublisherStats.java
##########
@@ -31,7 +29,9 @@
public double msgDropRate;
public NonPersistentPublisherStats add(NonPersistentPublisherStats stats) {
- checkNotNull(stats);
Review comment:
there is Objects.requireNonNull in the JDK, we replace Preconditions with Objects
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/FunctionStats.java
##########
@@ -18,25 +18,17 @@
*/
package org.apache.pulsar.common.policies.data;
-import com.fasterxml.jackson.annotation.JsonInclude;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.annotation.JsonPropertyOrder;
-import java.io.IOException;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import lombok.Data;
import lombok.EqualsAndHashCode;
-import org.apache.pulsar.common.util.ObjectMapperFactory;
/**
* Statistics for Pulsar Function.
*/
@Data
-@JsonInclude(JsonInclude.Include.ALWAYS)
-@JsonPropertyOrder({"receivedTotal", "processedSuccessfullyTotal", "systemExceptionsTotal", "userExceptionsTotal",
Review comment:
is this removal changing the final output of the JSON encoded version of this structure ?
--
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 #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
freeznet commented on a change in pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#discussion_r642085162
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/NonPersistentPublisherStats.java
##########
@@ -31,7 +29,9 @@
public double msgDropRate;
public NonPersistentPublisherStats add(NonPersistentPublisherStats stats) {
- checkNotNull(stats);
Review comment:
thanks for your suggestion, applied with https://github.com/apache/pulsar/pull/10697/commits/4b123ae39b32151bc6349a51037469184fe82f96
--
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 #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
freeznet commented on a change in pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#discussion_r642085509
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Bookies.java
##########
@@ -31,12 +32,12 @@
/**
* Gets the rack placement information for all the bookies in the cluster.
*/
- BookiesRackConfiguration getBookiesRackInfo() throws PulsarAdminException;
+ TreeMap<String, Map<String, BookieInfo>> getBookiesRackInfo() throws PulsarAdminException;
Review comment:
I have changed it back to `BookiesRackConfiguration`.
--
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 #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#issuecomment-851369886
@eolivelli thanks for your comments. I just have a discussion with @codelipenghui earlier today, and we are on the same page. I will push some commits to change the class name, like change `ClusterData` to `ClusterDataImpl` as the implementation, and change `ClusterDataInterface` to `ClusterData` as the interface.
--
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 #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
freeznet commented on a change in pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#discussion_r642085025
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/FunctionStats.java
##########
@@ -18,25 +18,17 @@
*/
package org.apache.pulsar.common.policies.data;
-import com.fasterxml.jackson.annotation.JsonInclude;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.annotation.JsonPropertyOrder;
-import java.io.IOException;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import lombok.Data;
import lombok.EqualsAndHashCode;
-import org.apache.pulsar.common.util.ObjectMapperFactory;
/**
* Statistics for Pulsar Function.
*/
@Data
-@JsonInclude(JsonInclude.Include.ALWAYS)
-@JsonPropertyOrder({"receivedTotal", "processedSuccessfullyTotal", "systemExceptionsTotal", "userExceptionsTotal",
Review comment:
My initial plan was to use MixIn class with jackson to add annotations but seems it is not working as I expected. So I extracted `FunctionStats` and inner classes with interfaces, please take another look on this commit https://github.com/apache/pulsar/pull/10697/commits/80fb31cab755915ef95034d2e43070f2249c130e, thanks.
--
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 #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
freeznet closed pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697
--
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] codelipenghui commented on a change in pull request #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#discussion_r642291197
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/ProxyStats.java
##########
@@ -33,13 +30,13 @@
* @throws PulsarAdminException
*/
- JsonArray getConnections() throws PulsarAdminException;
Review comment:
Since we have discussed in the community meeting, here need to introduce breaking change to keep the API module simple, I agree with using string the returned type, it would be better if we can add a description to indicate the return string in a JSON format.
--
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 #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
freeznet commented on a change in pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#discussion_r642085663
##########
File path: pulsar-client-admin-api/pom.xml
##########
@@ -36,13 +36,32 @@
<dependency>
<groupId>${project.groupId}</groupId>
- <artifactId>pulsar-common</artifactId>
- <version>${project.parent.version}</version>
+ <artifactId>pulsar-client-api</artifactId>
+ <version>${project.version}</version>
</dependency>
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>io.swagger</groupId>
+ <artifactId>swagger-annotations</artifactId>
+ <scope>provided</scope>
Review comment:
@merlimat thanks for your comments, I have removed swagger dependency from `pulsar-client-admin-api` here and convert related classes into interfaces, PTAL, thanks.
--
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] eolivelli commented on pull request #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#issuecomment-852040257
> 1. Actually, I do not get this point very clear, it would be great if you could give me more background about how you came up with this idea?
You can try to write a simple client program that uses the new Admin API without using the "Impl" classes.
If we model the POJOs/Beans with interfaces then the user may try to use a custom implementation of the interface.
Having interfaces makes sense for the users who "read" data, actually the implementation will be provided by the runtime.
But when you come to use APIs (like createCluster) that need an implementation of the interface then you are stuck in using something that is not part of the "public API".
The Public API must be the only thing that you need in order to use the API.
`createCluster` wants an implementation of `ClusterData` but you cannot provide it if you do not use the *Impl classes.
I am assuming that the *Impl classes are "implementation details", and must not used explicitly but the client.
otherwise we are not really splitting the API in some "API part" (public) and "Implementation part" (hidden, not usable directly)
if we want to go this way we must provide builders or factories for all of the new interfaces, like we do with the Schema API (for instance Schema.AUTO_CONSUME());
Apart from this problem, this is a big breaking change, that we should do with care, and probably it is not safe to do it for 2.8.0 and we have to postpone it to 2.9.0 (or even 3.0.0??)
I believe the this is one of the cases where a PIP is a good way of working (the community meeting is informal and not everyone is able to attend)
--
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] codelipenghui commented on a change in pull request #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#discussion_r642278059
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Namespaces.java
##########
@@ -3681,7 +3681,7 @@ void setIsAllowAutoUpdateSchema(String namespace, boolean isAllowAutoUpdateSchem
* @throws PulsarAdminException
* Unexpected error
*/
- void setOffloadPolicies(String namespace, OffloadPolicies offloadPolicies) throws PulsarAdminException;
Review comment:
This will also be a break change.
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/BrokerStats.java
##########
@@ -36,15 +34,15 @@
* @throws PulsarAdminException
*/
- JsonArray getMetrics() throws PulsarAdminException;
Review comment:
Will be a breaking change if we change the interface
--
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 #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#issuecomment-851553077
@merlimat @codelipenghui @eolivelli please help to review this PR if you have time, thanks.
--
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] eolivelli commented on pull request #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#issuecomment-852240303
Can we add some builder/factory methods in order to help user code to create instances of the new interfaces without depending on the *Impl classes.
Alternatively we could change the name for instance from ClusterDataImpl to ClusterDataBean, following Java naming conventions and state Javadocs how to use them
--
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 #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#issuecomment-851140707
/pulsarbot run-failure-checks
--
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 #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#issuecomment-851232299
/pulsarbot run-failure-checks
--
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 #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#discussion_r638936244
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Bookies.java
##########
@@ -31,12 +32,12 @@
/**
* Gets the rack placement information for all the bookies in the cluster.
*/
- BookiesRackConfiguration getBookiesRackInfo() throws PulsarAdminException;
+ TreeMap<String, Map<String, BookieInfo>> getBookiesRackInfo() throws PulsarAdminException;
Review comment:
Why can't we also bring over the `BookiesRackConfiguration` class? It doesn't look to have any dependencies.
##########
File path: pulsar-client-admin-api/pom.xml
##########
@@ -36,13 +36,32 @@
<dependency>
<groupId>${project.groupId}</groupId>
- <artifactId>pulsar-common</artifactId>
- <version>${project.parent.version}</version>
+ <artifactId>pulsar-client-api</artifactId>
+ <version>${project.version}</version>
</dependency>
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>io.swagger</groupId>
+ <artifactId>swagger-annotations</artifactId>
+ <scope>provided</scope>
Review comment:
I'm ok if this was a step of the overall changes, though in the final form we cannot have these provided dependencies. This just removes the dependencies for the user, but then the error will be at runtime, especially since these symbols will be then shaded.
##########
File path: pulsar-client-admin-api/pom.xml
##########
@@ -36,13 +36,32 @@
<dependency>
<groupId>${project.groupId}</groupId>
- <artifactId>pulsar-common</artifactId>
- <version>${project.parent.version}</version>
+ <artifactId>pulsar-client-api</artifactId>
+ <version>${project.version}</version>
</dependency>
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>io.swagger</groupId>
+ <artifactId>swagger-annotations</artifactId>
+ <scope>provided</scope>
Review comment:
Since we cannot take out the Swagger annotations, that was the reason I was suggesting to convert the POJO classes into interfaces and have the Swagger annotations on the implementation only.
--
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 #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#issuecomment-851433535
/pulsarbot run-failure-checks
--
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] codelipenghui commented on a change in pull request #10697: [fix #9640] remove `pulsar-client-admin-api` dependency : `pulsar-common`
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #10697:
URL: https://github.com/apache/pulsar/pull/10697#discussion_r642275231
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/BrokerStats.java
##########
@@ -36,15 +34,15 @@
* @throws PulsarAdminException
*/
- JsonArray getMetrics() throws PulsarAdminException;
Review comment:
Will be a breaking change if we change the interface
--
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