[GitHub] [pulsar] tuteng commented on a change in pull request #5767: Support batch authorization of partitioned topic
tuteng commented on a change in pull request #5767: Support batch authorization
of partitioned topic
URL: https://github.com/apache/pulsar/pull/5767#discussion_r352318147
##
File path:
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
##
@@ -312,4 +316,74 @@ public void testGetPartitionedTopicsList() throws
KeeperException, InterruptedEx
Assert.assertEquals(nonPersistentPartitionedTopics.size(), 1);
Assert.assertEquals(TopicName.get(nonPersistentPartitionedTopics.get(0)).getDomain().value(),
TopicDomain.non_persistent.value());
}
+
+@Test
+public void testGrantNonPartitionedTopic() {
+final String topicName = "non-partitioned-topic";
+persistentTopics.createNonPartitionedTopic(testTenant, testNamespace,
topicName, true);
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
topicName, role, expectActions);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace, topicName);
+Assert.assertEquals(permissions.get(role), expectActions);
+}
+
+@Test
+public void testGrantPartitionedTopic() {
+final String partitionedTopicName = "partitioned-topic";
+final int numPartitions = 5;
+persistentTopics.createPartitionedTopic(testTenant, testNamespace,
partitionedTopicName, numPartitions);
+
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
partitionedTopicName, role, expectActions);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace,
+partitionedTopicName);
+Assert.assertEquals(permissions.get(role), expectActions);
+TopicName topicName = TopicName.get(partitionedTopicName);
+for (int i = 0; i < numPartitions; i++) {
+TopicName partition = topicName.getPartition(i);
+Map> partitionPermissions =
persistentTopics.getPermissionsOnTopic(testTenant,
Review comment:
`partition` here is a full-path topic, so the splice is wrong.
The following wrong path will be spliced here:
```
persistent://my-tenant/my-namespace/persistent://my-tenant/my-namespace/partitioned-topic-partition-0
persistent://my-tenant/my-namespace/persistent://my-tenant/my-namespace/partitioned-topic-partition-1
persistent://my-tenant/my-namespace/persistent://my-tenant/my-namespace/partitioned-topic-partition-2
persistent://my-tenant/my-namespace/persistent://my-tenant/my-namespace/partitioned-topic-partition-3
persistent://my-tenant/my-namespace/persistent://my-tenant/my-namespace/partitioned-topic-partition-4
```
The correct path should look like this:
```
persistent://my-tenant/my-namespace/partitioned-topic-partition-0
persistent://my-tenant/my-namespace/partitioned-topic-partition-1
persistent://my-tenant/my-namespace/partitioned-topic-partition-2
persistent://my-tenant/my-namespace/partitioned-topic-partition-3
persistent://my-tenant/my-namespace/partitioned-topic-partition-4
```
```
public TopicName getPartition(int index) {
if (index == -1 ||
this.toString().contains(PARTITIONED_TOPIC_SUFFIX)) {
return this;
}
String partitionName = this.toString() + PARTITIONED_TOPIC_SUFFIX +
index;
return get(partitionName);
}
@Override
public String toString() {
return completeTopicName;
}
if (isV2()) {
this.completeTopicName = String.format("%s://%s/%s/%s",
domain, tenant,
namespacePortion, localName);
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] tuteng commented on a change in pull request #5767: Support batch authorization of partitioned topic
tuteng commented on a change in pull request #5767: Support batch authorization
of partitioned topic
URL: https://github.com/apache/pulsar/pull/5767#discussion_r352318147
##
File path:
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
##
@@ -312,4 +316,74 @@ public void testGetPartitionedTopicsList() throws
KeeperException, InterruptedEx
Assert.assertEquals(nonPersistentPartitionedTopics.size(), 1);
Assert.assertEquals(TopicName.get(nonPersistentPartitionedTopics.get(0)).getDomain().value(),
TopicDomain.non_persistent.value());
}
+
+@Test
+public void testGrantNonPartitionedTopic() {
+final String topicName = "non-partitioned-topic";
+persistentTopics.createNonPartitionedTopic(testTenant, testNamespace,
topicName, true);
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
topicName, role, expectActions);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace, topicName);
+Assert.assertEquals(permissions.get(role), expectActions);
+}
+
+@Test
+public void testGrantPartitionedTopic() {
+final String partitionedTopicName = "partitioned-topic";
+final int numPartitions = 5;
+persistentTopics.createPartitionedTopic(testTenant, testNamespace,
partitionedTopicName, numPartitions);
+
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
partitionedTopicName, role, expectActions);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace,
+partitionedTopicName);
+Assert.assertEquals(permissions.get(role), expectActions);
+TopicName topicName = TopicName.get(partitionedTopicName);
+for (int i = 0; i < numPartitions; i++) {
+TopicName partition = topicName.getPartition(i);
+Map> partitionPermissions =
persistentTopics.getPermissionsOnTopic(testTenant,
Review comment:
`partition` here is a full-path topic, so the splice is wrong.
The following wrong path will be spliced here:
```
persistent://my-tenant/my-namespace/persistent://my-tenant/my-namespace/partitioned-topic-partition-0
persistent://my-tenant/my-namespace/persistent://my-tenant/my-namespace/partitioned-topic-partition-1
persistent://my-tenant/my-namespace/persistent://my-tenant/my-namespace/partitioned-topic-partition-2
persistent://my-tenant/my-namespace/persistent://my-tenant/my-namespace/partitioned-topic-partition-3
persistent://my-tenant/my-namespace/persistent://my-tenant/my-namespace/partitioned-topic-partition-4
```
The correct path should look like this:
```
persistent://my-tenant/my-namespace/partitioned-topic-partition-0
persistent://my-tenant/my-namespace/partitioned-topic-partition-1
persistent://my-tenant/my-namespace/partitioned-topic-partition-2
persistent://my-tenant/my-namespace/partitioned-topic-partition-3
persistent://my-tenant/my-namespace/partitioned-topic-partition-4
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] tuteng commented on a change in pull request #5767: Support batch authorization of partitioned topic
tuteng commented on a change in pull request #5767: Support batch authorization
of partitioned topic
URL: https://github.com/apache/pulsar/pull/5767#discussion_r352317912
##
File path:
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
##
@@ -312,4 +316,82 @@ public void testGetPartitionedTopicsList() throws
KeeperException, InterruptedEx
Assert.assertEquals(nonPersistentPartitionedTopics.size(), 1);
Assert.assertEquals(TopicName.get(nonPersistentPartitionedTopics.get(0)).getDomain().value(),
TopicDomain.non_persistent.value());
}
+
+@Test
+public void testGrantNonPartitionedTopic() {
+final String topicName = "non-partitioned-topic";
+persistentTopics.createNonPartitionedTopic(testTenant, testNamespace,
topicName, true);
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
topicName, role, expectActions);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace, topicName);
+Assert.assertEquals(permissions.get(role), expectActions);
+}
+
+@Test
+public void testGrantPartitionedTopic() {
+final String partitionedTopicName = "partitioned-topic";
+final int numPartitions = 5;
+LocalZooKeeperCacheService mockLocalZooKeeperCacheService =
mock(LocalZooKeeperCacheService.class);
+ZooKeeperChildrenCache mockZooKeeperChildrenCache =
mock(ZooKeeperChildrenCache.class);
+
doReturn(mockLocalZooKeeperCacheService).when(pulsar).getLocalZkCacheService();
+
doReturn(mockZooKeeperChildrenCache).when(mockLocalZooKeeperCacheService).managedLedgerListCache();
Review comment:
First of all, this logic has no problem with non-partitioned topics.
Let's look at the creation of the partitioned topic first. When we use the
following command to create a partitioned topic,
```
./bin/pulsar-admin topics create-partitioned-topic test-topic -p 2
```
we will eventually generate the following topic in the zookeeper.
```
persistent://public/default/test-topic-partition-0
persistent://public/default/test-topic-partition-1
```
When we use the `./bin/pulsar-admin topics grant-permissions` command to
authorize for a partitioned topic, we are actually authorizing these topics:
```
persistent://public/default/test-topic-partition-0
persistent://public/default/test-topic-partition-1
```
Therefore, at present, when we get permission, we also need to provide the
following topic name:
```
persistent://public/default/test-topic-partition-0
persistent://public/default/test-topic-partition-1
```
However, in actual use, users prefer to provide the following name to get
all the permissions under the topic.
```
persistent://public/default/test-topic
```
So we hope that if you have time or are interested, you can provide this
support in the `internalGetPermissionsOnTopic` function.
```
if the topic is partitioned topic
Loop to get permissions for each topic
else
Logic unchanged
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] zhaohaidao commented on issue #5767: Support batch authorization of partitioned topic
zhaohaidao commented on issue #5767: Support batch authorization of partitioned topic URL: https://github.com/apache/pulsar/pull/5767#issuecomment-560044634 > > I thought a draft pr would not be visible before. > > @zhaohaidao it is a good practice to add description (even it is a draft PR), so that people in the community knows the context of the pull request. Thanks for your advice. I will follow best practices in the future pr. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] zhaohaidao commented on a change in pull request #5767: Support batch authorization of partitioned topic
zhaohaidao commented on a change in pull request #5767: Support batch
authorization of partitioned topic
URL: https://github.com/apache/pulsar/pull/5767#discussion_r352316028
##
File path:
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
##
@@ -312,4 +316,82 @@ public void testGetPartitionedTopicsList() throws
KeeperException, InterruptedEx
Assert.assertEquals(nonPersistentPartitionedTopics.size(), 1);
Assert.assertEquals(TopicName.get(nonPersistentPartitionedTopics.get(0)).getDomain().value(),
TopicDomain.non_persistent.value());
}
+
+@Test
+public void testGrantNonPartitionedTopic() {
+final String topicName = "non-partitioned-topic";
+persistentTopics.createNonPartitionedTopic(testTenant, testNamespace,
topicName, true);
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
topicName, role, expectActions);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace, topicName);
+Assert.assertEquals(permissions.get(role), expectActions);
+}
+
+@Test
+public void testGrantPartitionedTopic() {
+final String partitionedTopicName = "partitioned-topic";
+final int numPartitions = 5;
+LocalZooKeeperCacheService mockLocalZooKeeperCacheService =
mock(LocalZooKeeperCacheService.class);
+ZooKeeperChildrenCache mockZooKeeperChildrenCache =
mock(ZooKeeperChildrenCache.class);
+
doReturn(mockLocalZooKeeperCacheService).when(pulsar).getLocalZkCacheService();
+
doReturn(mockZooKeeperChildrenCache).when(mockLocalZooKeeperCacheService).managedLedgerListCache();
Review comment:
Hi, I went through the grant and get logic for permissions and It seems
current logic support get permissions for partitions of a topic. Pls help me
check if my understanding is right.
The permissions for partitions will be stored in a map named
destination_auth, the same as the parent topic of partitions.
```java
private void grantPermissions(String topicUri, String role,
Set actions) {
try {
...
Policies policies = jsonMapper().readValue(content,
Policies.class);
if
(!policies.auth_policies.destination_auth.containsKey(topicUri)) {
policies.auth_policies.destination_auth.put(topicUri, new
TreeMap>());
}
policies.auth_policies.destination_auth.get(topicUri).put(role,
actions);
// Write the new policies to zookeeper
globalZk().setData(path(POLICIES, namespaceName.toString()),
jsonMapper().writeValueAsBytes(policies),
nodeStat.getVersion());
...
}
```
Then get permissions logic for a partition: try to get permissions from
auth_policies.destination_auth directly by topicUri.
```java
protected Map> internalGetPermissionsOnTopic() {
// This operation should be reading from zookeeper and it should be
allowed without having admin privileges
validateAdminAccessForTenant(namespaceName.getTenant());
String topicUri = topicName.toString();
try {
...
// Then add topic level permissions
if (auth.destination_auth.containsKey(topicUri)) {
for (Map.Entry> entry :
auth.destination_auth.get(topicUri).entrySet()) {
String role = entry.getKey();
Set topicPermissions = entry.getValue();
if (!permissions.containsKey(role)) {
permissions.put(role, topicPermissions);
} else {
// Do the union between namespace and topic level
Set union =
Sets.union(permissions.get(role), topicPermissions);
permissions.put(role, union);
}
}
}
return permissions;
} catch (Exception e) {
...
}
}
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] zhaohaidao commented on a change in pull request #5767: Support batch authorization of partitioned topic
zhaohaidao commented on a change in pull request #5767: Support batch
authorization of partitioned topic
URL: https://github.com/apache/pulsar/pull/5767#discussion_r352316028
##
File path:
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
##
@@ -312,4 +316,82 @@ public void testGetPartitionedTopicsList() throws
KeeperException, InterruptedEx
Assert.assertEquals(nonPersistentPartitionedTopics.size(), 1);
Assert.assertEquals(TopicName.get(nonPersistentPartitionedTopics.get(0)).getDomain().value(),
TopicDomain.non_persistent.value());
}
+
+@Test
+public void testGrantNonPartitionedTopic() {
+final String topicName = "non-partitioned-topic";
+persistentTopics.createNonPartitionedTopic(testTenant, testNamespace,
topicName, true);
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
topicName, role, expectActions);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace, topicName);
+Assert.assertEquals(permissions.get(role), expectActions);
+}
+
+@Test
+public void testGrantPartitionedTopic() {
+final String partitionedTopicName = "partitioned-topic";
+final int numPartitions = 5;
+LocalZooKeeperCacheService mockLocalZooKeeperCacheService =
mock(LocalZooKeeperCacheService.class);
+ZooKeeperChildrenCache mockZooKeeperChildrenCache =
mock(ZooKeeperChildrenCache.class);
+
doReturn(mockLocalZooKeeperCacheService).when(pulsar).getLocalZkCacheService();
+
doReturn(mockZooKeeperChildrenCache).when(mockLocalZooKeeperCacheService).managedLedgerListCache();
Review comment:
Hi, I went through the grant and get logic for permissions and It seems
current logic support get permissions for partitions of a topic. Pls help me
check if my understanding is right.
The permissions will be stored in a map named destination_auth as followed.
```java
private void grantPermissions(String topicUri, String role,
Set actions) {
try {
...
Policies policies = jsonMapper().readValue(content,
Policies.class);
if
(!policies.auth_policies.destination_auth.containsKey(topicUri)) {
policies.auth_policies.destination_auth.put(topicUri, new
TreeMap>());
}
policies.auth_policies.destination_auth.get(topicUri).put(role,
actions);
// Write the new policies to zookeeper
globalZk().setData(path(POLICIES, namespaceName.toString()),
jsonMapper().writeValueAsBytes(policies),
nodeStat.getVersion());
...
}
```
Then get permissions logic for a partition: try to get permissions from
auth_policies.destination_auth directly by topic_name.
```java
protected Map> internalGetPermissionsOnTopic() {
// This operation should be reading from zookeeper and it should be
allowed without having admin privileges
validateAdminAccessForTenant(namespaceName.getTenant());
String topicUri = topicName.toString();
try {
...
// Then add topic level permissions
if (auth.destination_auth.containsKey(topicUri)) {
for (Map.Entry> entry :
auth.destination_auth.get(topicUri).entrySet()) {
String role = entry.getKey();
Set topicPermissions = entry.getValue();
if (!permissions.containsKey(role)) {
permissions.put(role, topicPermissions);
} else {
// Do the union between namespace and topic level
Set union =
Sets.union(permissions.get(role), topicPermissions);
permissions.put(role, union);
}
}
}
return permissions;
} catch (Exception e) {
...
}
}
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] codelipenghui commented on issue #5680: [transaction-coordinator] Implementation of transaction coordinator client.
codelipenghui commented on issue #5680: [transaction-coordinator] Implementation of transaction coordinator client. URL: https://github.com/apache/pulsar/pull/5680#issuecomment-560035515 run cpp tests 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] codelipenghui commented on issue #5739: Use message bytes instead of message number for producer pending messages
codelipenghui commented on issue #5739: Use message bytes instead of message number for producer pending messages URL: https://github.com/apache/pulsar/issues/5739#issuecomment-560035238 welcome @zhaohaidao and looking forward to your contribution. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] codelipenghui commented on issue #5702: [pulsar-sql]Expose configurations of managed ledger and bookkeeper client.
codelipenghui commented on issue #5702: [pulsar-sql]Expose configurations of managed ledger and bookkeeper client. URL: https://github.com/apache/pulsar/pull/5702#issuecomment-560034943 This PR need to fix the integration tests, i have checked with gaoran, there are problems when we disable entry cache and using pulsar sql to retrieve data while using publish_time to trim entries, gaoran will fix it soon 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] codelipenghui commented on issue #5734: [Reopen][Issue 5597]retry when getPartitionedTopicMetadata failed
codelipenghui commented on issue #5734: [Reopen][Issue 5597]retry when getPartitionedTopicMetadata failed URL: https://github.com/apache/pulsar/pull/5734#issuecomment-560034625 run java8 tests 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] codelipenghui commented on issue #5765: add Murmur3_32Hash private constructor
codelipenghui commented on issue #5765: add Murmur3_32Hash private constructor URL: https://github.com/apache/pulsar/pull/5765#issuecomment-560034357 run java8 tests 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] codelipenghui commented on issue #5776: fix potential NPE and repeated conditional test
codelipenghui commented on issue #5776: fix potential NPE and repeated conditional test URL: https://github.com/apache/pulsar/pull/5776#issuecomment-560034327 run java8 tests 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar-client-go] cckellogg commented on a change in pull request #109: Fix subscriber bug and use Consumer interface for multi topic consumer.
cckellogg commented on a change in pull request #109: Fix subscriber bug and
use Consumer interface for multi topic consumer.
URL: https://github.com/apache/pulsar-client-go/pull/109#discussion_r352298820
##
File path: pulsar/consumer_multitopic.go
##
@@ -45,54 +45,32 @@ func newMultiTopicConsumer(client *client, options
ConsumerOptions, topics []str
mtc := {
options: options,
messageCh: messageCh,
- consumers: make(map[string]*consumer, len(topics)),
+ consumers: make(map[string]Consumer, len(topics)),
closeCh: make(chan struct{}),
log: {},
}
- type ConsumerError struct {
- err error
- topicstring
- consumer *consumer
- }
-
- var wg sync.WaitGroup
- wg.Add(len(topics))
- ch := make(chan ConsumerError, len(topics))
- for i := range topics {
- go func(t string) {
- defer wg.Done()
- c, err := internalTopicSubscribe(client, options, t,
messageCh)
- ch <- ConsumerError{
- err: err,
- topic:t,
- consumer: c,
- }
- }(topics[i])
- }
-
- go func() {
- wg.Wait()
- close(ch)
- }()
-
var errs error
- for ce := range ch {
+ consumers := make(map[string]Consumer, len(topics))
Review comment:
That's a good question. The idea was to only set the consumers when
everything succeeded but I don't think it matters since we return nil on
failure. I'll remove it.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] massakam opened a new pull request #5777: [pulsar-client] Fix bug that beforeConsume() of interceptor is not called when receiver queue size is 0
massakam opened a new pull request #5777: [pulsar-client] Fix bug that beforeConsume() of interceptor is not called when receiver queue size is 0 URL: https://github.com/apache/pulsar/pull/5777 ### Motivation I implemented `ConsumerInterceptor` and used it, but I noticed that the `beforeConsume()` method is not executed when the size of the receiver queue is 0. ### Modifications If the receiver queue size is 0, an instance of `ZeroQueueConsumerImpl` class is created instead of `ConsumerImpl`, but `beforeConsume()` is not called in this class now. So I fixed `ZeroQueueConsumerImpl`. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] jiazhai commented on issue #5771: [docs] Fix link issue for "Authentication and authorization in Pulsar"
jiazhai commented on issue #5771: [docs] Fix link issue for "Authentication and authorization in Pulsar" URL: https://github.com/apache/pulsar/pull/5771#issuecomment-559970323 run java8 tests 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] jiazhai commented on issue #5776: fix potential NPE and repeated conditional test
jiazhai commented on issue #5776: fix potential NPE and repeated conditional test URL: https://github.com/apache/pulsar/pull/5776#issuecomment-559970063 run java8 tests 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] jiazhai commented on issue #5765: add Murmur3_32Hash private constructor
jiazhai commented on issue #5765: add Murmur3_32Hash private constructor URL: https://github.com/apache/pulsar/pull/5765#issuecomment-559970017 run java8 tests 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: us...@infra.apache.org With regards, Apache Git Services
[pulsar] branch master updated (f0b34e7 -> 163aa43)
This is an automated email from the ASF dual-hosted git repository. guangning pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/pulsar.git. from f0b34e7 Bump Lombok version to allow building with a JDK12>=12 (#5772) (#5773) add 163aa43 Fix pulsar-manager download link (#5774) No new revisions were added by this update. Summary of changes: site2/website/pages/en/download.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
[GitHub] [pulsar] tuteng merged pull request #5774: [website]Fix pulsar-manager download link
tuteng merged pull request #5774: [website]Fix pulsar-manager download link URL: https://github.com/apache/pulsar/pull/5774 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] tuteng commented on a change in pull request #5767: Support batch authorization of partitioned topic
tuteng commented on a change in pull request #5767: Support batch authorization
of partitioned topic
URL: https://github.com/apache/pulsar/pull/5767#discussion_r352285215
##
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##
@@ -301,13 +295,29 @@ protected void internalGrantPermissionsOnTopic(String
role, Set acti
log.warn("[{}] Failed to grant permissions on topic {}: concurrent
modification", clientAppId(),
topicUri);
throw new RestException(Status.CONFLICT, "Concurrent
modification");
-}
-catch (Exception e) {
+} catch (Exception e) {
log.error("[{}] Failed to grant permissions for topic {}",
clientAppId(), topicUri, e);
throw new RestException(e);
}
}
+protected void internalGrantPermissionsOnTopic(String role,
Set actions) {
+// This operation should be reading from zookeeper and it should be
allowed without having admin privileges
+validateAdminAccessForTenant(namespaceName.getTenant());
+validatePoliciesReadOnlyAccess();
+
+PartitionedTopicMetadata meta = getPartitionedTopicMetadata(topicName,
true, false);
Review comment:
Before that, we seem to add a version of judgment. I'm not sure whether the
partition topic is supported in the v1 version because the v1 version of the
domain path has `cluster` attribute, and the v2 version does not have this
attribute, So calling function `getPartitionedTopicMetadata` directly will
throw an exception. therefore, I think it may need to add a version check here.
```
if (topicName.isV2()) {
getPartitionedTopicMetadata(topicName, true, false);
// Auth to parititioned topic
} else {
// Non-partitioned topic normal authorization
}
```
REST API v1:
```
persistent://tenant/cluster-name/namespace/topic-name
```
REST API v2
```
persistent://tenant/namespace/topic-name
```
@sijie I'd like to hear your opinion. I don't know whether rest API v1
supports partitioned topic or not.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] tuteng commented on a change in pull request #5767: Support batch authorization of partitioned topic
tuteng commented on a change in pull request #5767: Support batch authorization
of partitioned topic
URL: https://github.com/apache/pulsar/pull/5767#discussion_r352285215
##
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##
@@ -301,13 +295,29 @@ protected void internalGrantPermissionsOnTopic(String
role, Set acti
log.warn("[{}] Failed to grant permissions on topic {}: concurrent
modification", clientAppId(),
topicUri);
throw new RestException(Status.CONFLICT, "Concurrent
modification");
-}
-catch (Exception e) {
+} catch (Exception e) {
log.error("[{}] Failed to grant permissions for topic {}",
clientAppId(), topicUri, e);
throw new RestException(e);
}
}
+protected void internalGrantPermissionsOnTopic(String role,
Set actions) {
+// This operation should be reading from zookeeper and it should be
allowed without having admin privileges
+validateAdminAccessForTenant(namespaceName.getTenant());
+validatePoliciesReadOnlyAccess();
+
+PartitionedTopicMetadata meta = getPartitionedTopicMetadata(topicName,
true, false);
Review comment:
Before that, we seem to add a version of judgment. I'm not sure whether the
partition topic is supported in the v1 version because the v1 version of the
domain path has `cluster` attribute, and the v2 version does not have this
attribute, So calling function `getPartitionedTopicMetadata` directly will
throw an exception. therefore, I think it may need to add a version check here.
```
if (topicName.isV2()) {
getPartitionedTopicMetadata(topicName, true, false);
// Auth to parititioned topic
} else {
// Non-partitioned topic normal authorization
}
```
REST API v1:
```
persistent://tenant/cluster-name/namespace/topic-name
```
REST API v2:
```
persistent://tenant/namespace/topic-name
```
@sijie I'd like to hear your opinion. I don't know whether rest API v1
supports partitioned topic or not.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] tuteng commented on a change in pull request #5767: Support batch authorization of partitioned topic
tuteng commented on a change in pull request #5767: Support batch authorization
of partitioned topic
URL: https://github.com/apache/pulsar/pull/5767#discussion_r352285204
##
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##
@@ -369,6 +373,24 @@ protected void internalRevokePermissionsOnTopic(String
role) {
log.error("[{}] Failed to revoke permissions for topic {}",
clientAppId(), topicUri, e);
throw new RestException(e);
}
+
+}
+
+protected void internalRevokePermissionsOnTopic(String role) {
+// This operation should be reading from zookeeper and it should be
allowed without having admin privileges
+validateAdminAccessForTenant(namespaceName.getTenant());
+validatePoliciesReadOnlyAccess();
+
+PartitionedTopicMetadata meta = getPartitionedTopicMetadata(topicName,
true, false);
Review comment:
Same as above.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] tuteng commented on a change in pull request #5767: Support batch authorization of partitioned topic
tuteng commented on a change in pull request #5767: Support batch authorization
of partitioned topic
URL: https://github.com/apache/pulsar/pull/5767#discussion_r352285631
##
File path:
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
##
@@ -312,4 +316,82 @@ public void testGetPartitionedTopicsList() throws
KeeperException, InterruptedEx
Assert.assertEquals(nonPersistentPartitionedTopics.size(), 1);
Assert.assertEquals(TopicName.get(nonPersistentPartitionedTopics.get(0)).getDomain().value(),
TopicDomain.non_persistent.value());
}
+
+@Test
+public void testGrantNonPartitionedTopic() {
+final String topicName = "non-partitioned-topic";
+persistentTopics.createNonPartitionedTopic(testTenant, testNamespace,
topicName, true);
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
topicName, role, expectActions);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace, topicName);
+Assert.assertEquals(permissions.get(role), expectActions);
+}
+
+@Test
+public void testGrantPartitionedTopic() {
+final String partitionedTopicName = "partitioned-topic";
+final int numPartitions = 5;
+LocalZooKeeperCacheService mockLocalZooKeeperCacheService =
mock(LocalZooKeeperCacheService.class);
+ZooKeeperChildrenCache mockZooKeeperChildrenCache =
mock(ZooKeeperChildrenCache.class);
+
doReturn(mockLocalZooKeeperCacheService).when(pulsar).getLocalZkCacheService();
+
doReturn(mockZooKeeperChildrenCache).when(mockLocalZooKeeperCacheService).managedLedgerListCache();
+persistentTopics.createPartitionedTopic(testTenant, testNamespace,
partitionedTopicName, numPartitions);
+
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
partitionedTopicName, role, expectActions);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace,
+partitionedTopicName);
+Assert.assertEquals(permissions.get(role), expectActions);
+TopicName topicName=TopicName.get(partitionedTopicName);
+for (int i=0; i> partitionPermissions =
persistentTopics.getPermissionsOnTopic(testTenant,
+testNamespace, partition.toString());
+Assert.assertEquals(partitionPermissions.get(role), expectActions);
+}
+}
+
+@Test
+public void testRevokeNonPartitionedTopic() {
+final String topicName = "non-partitioned-topic";
+persistentTopics.createNonPartitionedTopic(testTenant, testNamespace,
topicName, true);
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
topicName, role, expectActions);
+persistentTopics.revokePermissionsOnTopic(testTenant, testNamespace,
topicName, role);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace, topicName);
+Assert.assertEquals(permissions.get(role), null);
+}
+
+@Test
+public void testRevokePartitionedTopic() {
+final String partitionedTopicName = "partitioned-topic";
+final int numPartitions = 5;
+LocalZooKeeperCacheService mockLocalZooKeeperCacheService =
mock(LocalZooKeeperCacheService.class);
+ZooKeeperChildrenCache mockZooKeeperChildrenCache =
mock(ZooKeeperChildrenCache.class);
+
doReturn(mockLocalZooKeeperCacheService).when(pulsar).getLocalZkCacheService();
+
doReturn(mockZooKeeperChildrenCache).when(mockLocalZooKeeperCacheService).managedLedgerListCache();
+persistentTopics.createPartitionedTopic(testTenant, testNamespace,
partitionedTopicName, numPartitions);
+
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
partitionedTopicName, role, expectActions);
+persistentTopics.revokePermissionsOnTopic(testTenant, testNamespace,
partitionedTopicName, role);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace,
+partitionedTopicName);
+Assert.assertEquals(permissions.get(role), null);
+TopicName topicName=TopicName.get(partitionedTopicName);
+for (int i=0; i
[GitHub] [pulsar] tuteng commented on a change in pull request #5767: Support batch authorization of partitioned topic
tuteng commented on a change in pull request #5767: Support batch authorization
of partitioned topic
URL: https://github.com/apache/pulsar/pull/5767#discussion_r352285560
##
File path:
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
##
@@ -312,4 +316,82 @@ public void testGetPartitionedTopicsList() throws
KeeperException, InterruptedEx
Assert.assertEquals(nonPersistentPartitionedTopics.size(), 1);
Assert.assertEquals(TopicName.get(nonPersistentPartitionedTopics.get(0)).getDomain().value(),
TopicDomain.non_persistent.value());
}
+
+@Test
+public void testGrantNonPartitionedTopic() {
+final String topicName = "non-partitioned-topic";
+persistentTopics.createNonPartitionedTopic(testTenant, testNamespace,
topicName, true);
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
topicName, role, expectActions);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace, topicName);
+Assert.assertEquals(permissions.get(role), expectActions);
+}
+
+@Test
+public void testGrantPartitionedTopic() {
+final String partitionedTopicName = "partitioned-topic";
+final int numPartitions = 5;
+LocalZooKeeperCacheService mockLocalZooKeeperCacheService =
mock(LocalZooKeeperCacheService.class);
+ZooKeeperChildrenCache mockZooKeeperChildrenCache =
mock(ZooKeeperChildrenCache.class);
+
doReturn(mockLocalZooKeeperCacheService).when(pulsar).getLocalZkCacheService();
+
doReturn(mockZooKeeperChildrenCache).when(mockLocalZooKeeperCacheService).managedLedgerListCache();
+persistentTopics.createPartitionedTopic(testTenant, testNamespace,
partitionedTopicName, numPartitions);
+
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
partitionedTopicName, role, expectActions);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace,
+partitionedTopicName);
+Assert.assertEquals(permissions.get(role), expectActions);
+TopicName topicName=TopicName.get(partitionedTopicName);
+for (int i=0; i> partitionPermissions =
persistentTopics.getPermissionsOnTopic(testTenant,
+testNamespace, partition.toString());
+Assert.assertEquals(partitionPermissions.get(role), expectActions);
+}
+}
+
+@Test
+public void testRevokeNonPartitionedTopic() {
+final String topicName = "non-partitioned-topic";
+persistentTopics.createNonPartitionedTopic(testTenant, testNamespace,
topicName, true);
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
topicName, role, expectActions);
+persistentTopics.revokePermissionsOnTopic(testTenant, testNamespace,
topicName, role);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace, topicName);
+Assert.assertEquals(permissions.get(role), null);
+}
+
+@Test
+public void testRevokePartitionedTopic() {
+final String partitionedTopicName = "partitioned-topic";
+final int numPartitions = 5;
+LocalZooKeeperCacheService mockLocalZooKeeperCacheService =
mock(LocalZooKeeperCacheService.class);
+ZooKeeperChildrenCache mockZooKeeperChildrenCache =
mock(ZooKeeperChildrenCache.class);
+
doReturn(mockLocalZooKeeperCacheService).when(pulsar).getLocalZkCacheService();
+
doReturn(mockZooKeeperChildrenCache).when(mockLocalZooKeeperCacheService).managedLedgerListCache();
Review comment:
The same above.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] tuteng commented on a change in pull request #5767: Support batch authorization of partitioned topic
tuteng commented on a change in pull request #5767: Support batch authorization
of partitioned topic
URL: https://github.com/apache/pulsar/pull/5767#discussion_r35228
##
File path:
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
##
@@ -312,4 +316,82 @@ public void testGetPartitionedTopicsList() throws
KeeperException, InterruptedEx
Assert.assertEquals(nonPersistentPartitionedTopics.size(), 1);
Assert.assertEquals(TopicName.get(nonPersistentPartitionedTopics.get(0)).getDomain().value(),
TopicDomain.non_persistent.value());
}
+
+@Test
+public void testGrantNonPartitionedTopic() {
+final String topicName = "non-partitioned-topic";
+persistentTopics.createNonPartitionedTopic(testTenant, testNamespace,
topicName, true);
+String role = "role";
+Set expectActions = new HashSet<>();
+expectActions.add(AuthAction.produce);
+persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
topicName, role, expectActions);
+Map> permissions =
persistentTopics.getPermissionsOnTopic(testTenant, testNamespace, topicName);
+Assert.assertEquals(permissions.get(role), expectActions);
+}
+
+@Test
+public void testGrantPartitionedTopic() {
+final String partitionedTopicName = "partitioned-topic";
+final int numPartitions = 5;
+LocalZooKeeperCacheService mockLocalZooKeeperCacheService =
mock(LocalZooKeeperCacheService.class);
+ZooKeeperChildrenCache mockZooKeeperChildrenCache =
mock(ZooKeeperChildrenCache.class);
+
doReturn(mockLocalZooKeeperCacheService).when(pulsar).getLocalZkCacheService();
+
doReturn(mockZooKeeperChildrenCache).when(mockLocalZooKeeperCacheService).managedLedgerListCache();
Review comment:
Mock is not required here.
The following code can be used to check:
```
@Test
public void testGrantPartitionedTopic() throws Exception {
final String partitionedTopicName = "partitioned-topic";
final int numPartitions = 5;
persistentTopics.createPartitionedTopic(testTenant, testNamespace,
partitionedTopicName, numPartitions);
String role = "role";
Set expectActions = new HashSet<>();
expectActions.add(AuthAction.produce);
persistentTopics.grantPermissionsOnTopic(testTenant, testNamespace,
partitionedTopicName, role, expectActions);
for (int i=0; i < numPartitions; i++) {
Map> partitionPermissions =
persistentTopics.getPermissionsOnTopic(testTenant,
testNamespace, partitionedTopicName + "-partition-" + i);
Assert.assertEquals(partitionPermissions.get(role),
expectActions);
}
}
```
The `getPermissionsOnTopic` function does not seem to support searching
partitioned topic yet. I think you can consider adding a feature to the
`getPermissionsOnTopic` function to support searching partition topic in this
pr or by opening a new pr.
If the `getPermissionsOnTopic` function supports finding partitioned topic,
you can directly call the `getPermissionsOnTopic` function to get the value of
permissions, so this loop is not needed.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] tuteng commented on a change in pull request #5767: Support batch authorization of partitioned topic
tuteng commented on a change in pull request #5767: Support batch authorization
of partitioned topic
URL: https://github.com/apache/pulsar/pull/5767#discussion_r352285215
##
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##
@@ -301,13 +295,29 @@ protected void internalGrantPermissionsOnTopic(String
role, Set acti
log.warn("[{}] Failed to grant permissions on topic {}: concurrent
modification", clientAppId(),
topicUri);
throw new RestException(Status.CONFLICT, "Concurrent
modification");
-}
-catch (Exception e) {
+} catch (Exception e) {
log.error("[{}] Failed to grant permissions for topic {}",
clientAppId(), topicUri, e);
throw new RestException(e);
}
}
+protected void internalGrantPermissionsOnTopic(String role,
Set actions) {
+// This operation should be reading from zookeeper and it should be
allowed without having admin privileges
+validateAdminAccessForTenant(namespaceName.getTenant());
+validatePoliciesReadOnlyAccess();
+
+PartitionedTopicMetadata meta = getPartitionedTopicMetadata(topicName,
true, false);
Review comment:
Before that, we seem to add a version of judgment. I'm not sure whether the
partition topic is supported in the v1 version because the v1 version of the
domain path has `cluster` attribute, and the v2 version does not have this
attribute, So calling function `getPartitionedTopicMetadata` directly will
throw an exception. therefore, I think it may need to add a version check here.
```
if (topicName.isV2()) {
getPartitionedTopicMetadata(topicName, true, false);
// Auth to parititioned topic
} else {
// Non-partitioned topic normal authorization
}
```
REST API v1:
```
persistent://tenant/cluster-name/namespace/topic-name
```
REST API
```
persistent://tenant/namespace/topic-name
```
@sijie I'd like to hear your opinion. I don't know whether rest API v1
supports partitioned topic or not.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar-client-go] reugn commented on a change in pull request #108: [Issue #107] SetWriteDeadline on connection
reugn commented on a change in pull request #108: [Issue #107] SetWriteDeadline
on connection
URL: https://github.com/apache/pulsar-client-go/pull/108#discussion_r352282213
##
File path: pulsar/internal/connection.go
##
@@ -309,7 +309,13 @@ func (c *connection) WriteData(data []byte) {
func (c *connection) internalWriteData(data []byte) {
c.log.Debug("Write data: ", len(data))
- if _, err := c.cnx.Write(data); err != nil {
+ err := c.cnx.SetWriteDeadline(time.Now().Add(time.Second * 15))
Review comment:
Makes sense. 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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar-client-go] reugn commented on a change in pull request #109: Fix subscriber bug and use Consumer interface for multi topic consumer.
reugn commented on a change in pull request #109: Fix subscriber bug and use
Consumer interface for multi topic consumer.
URL: https://github.com/apache/pulsar-client-go/pull/109#discussion_r352281715
##
File path: pulsar/consumer_multitopic.go
##
@@ -45,54 +45,32 @@ func newMultiTopicConsumer(client *client, options
ConsumerOptions, topics []str
mtc := {
options: options,
messageCh: messageCh,
- consumers: make(map[string]*consumer, len(topics)),
+ consumers: make(map[string]Consumer, len(topics)),
closeCh: make(chan struct{}),
log: {},
}
- type ConsumerError struct {
- err error
- topicstring
- consumer *consumer
- }
-
- var wg sync.WaitGroup
- wg.Add(len(topics))
- ch := make(chan ConsumerError, len(topics))
- for i := range topics {
- go func(t string) {
- defer wg.Done()
- c, err := internalTopicSubscribe(client, options, t,
messageCh)
- ch <- ConsumerError{
- err: err,
- topic:t,
- consumer: c,
- }
- }(topics[i])
- }
-
- go func() {
- wg.Wait()
- close(ch)
- }()
-
var errs error
- for ce := range ch {
+ consumers := make(map[string]Consumer, len(topics))
Review comment:
Why do we need temporary consumers value here?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] candlerb commented on issue #5370: sql worker on Ubuntu 18: libprocname.so failing, and failing JVM vendor check
candlerb commented on issue #5370: sql worker on Ubuntu 18: libprocname.so failing, and failing JVM vendor check URL: https://github.com/apache/pulsar/issues/5370#issuecomment-55993 @dramaPainter: pulsar 2.4.1 includes presto 0.206. Note that #5386 was closed without merging, and git head https://github.com/apache/pulsar/blob/master/pulsar-sql/presto-distribution/pom.xml still fetches presto 0.206. Therefore, unless this changes, the problem will still be in 2.4.2/2.5.0 when they are released. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] aahmed-se commented on issue #5642: Add Github workflow for gated checkin
aahmed-se commented on issue #5642: Add Github workflow for gated checkin URL: https://github.com/apache/pulsar/pull/5642#issuecomment-559937315 run java8 tests 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] dramaPainter commented on issue #5370: sql worker on Ubuntu 18: libprocname.so failing, and failing JVM vendor check
dramaPainter commented on issue #5370: sql worker on Ubuntu 18: libprocname.so failing, and failing JVM vendor check URL: https://github.com/apache/pulsar/issues/5370#issuecomment-559934397 does it fixed? i download the binary file pulsar 2.4.1 from offical page: https://pulsar.apache.org/download/ and i checked the presto version is still 0.206??? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] belinda-wong commented on issue #5764: [docs] Fix link issue for "Authentication and authorization in Pulsar"
belinda-wong commented on issue #5764: [docs] Fix link issue for "Authentication and authorization in Pulsar" URL: https://github.com/apache/pulsar/pull/5764#issuecomment-559923914 > @belinda-wong In most cases, we only edit files in the master version(in the `./site2/docs` folder), and the changes will take effect in the next release. Our main purpose is to maintain the master version. > If it's an important and urgent issue, and you want to apply it in the current version, for example, v2.4.1, you should apply the changes in the `./site2/website/versioned_docs/version-2.4.1` folder. That's why we create `versioned_docs` folder, and there are less files in those folders than that in the master. > The changes you've made involves the files in the master and in `version-2.4.1`, if you want to apply it in v2.4.1, you should update the file in the `version-2.4.1` folder as well. ok, I see. thank you for the clarify. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] belinda-wong opened a new pull request #5764: [docs] Fix link issue for "Authentication and authorization in Pulsar"
belinda-wong opened a new pull request #5764: [docs] Fix link issue for "Authentication and authorization in Pulsar" URL: https://github.com/apache/pulsar/pull/5764 Fixes #5753 [Doc] the dead link for TLS Authentication on Proxies ### Motivation The link is dead. ### Modifications Add ".md" to the link. That is: security-tls-authentication.md#on-proxies 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [pulsar] belinda-wong commented on issue #5764: [docs] Fix link issue for "Authentication and authorization in Pulsar"
belinda-wong commented on issue #5764: [docs] Fix link issue for "Authentication and authorization in Pulsar" URL: https://github.com/apache/pulsar/pull/5764#issuecomment-559923702 I am not familiar with GitHub. I must have closed this PR by accident. 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: us...@infra.apache.org With regards, Apache Git Services
