You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/12/04 19:37:53 UTC
[GitHub] [kafka] chris8099 opened a new pull request #9694: MINOR:Modify aclsResources, discard Collection
chris8099 opened a new pull request #9694:
URL: https://github.com/apache/kafka/pull/9694
Modify DescribeAclsResponse, remove Collection and change to Iterable,
Modify KafkaApis, aclsResources method
----------------------------------------------------------------
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] [kafka] chris8099 commented on pull request #9694: MINOR:This method does not use a collection,changing to Iterable saves the cost of creating a collection.
Posted by GitBox <gi...@apache.org>.
chris8099 commented on pull request #9694:
URL: https://github.com/apache/kafka/pull/9694#issuecomment-739684136
@chia7712 Okay, please check 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] [kafka] chia7712 commented on pull request #9694: MINOR:Modify aclsResources, discard Collection
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9694:
URL: https://github.com/apache/kafka/pull/9694#issuecomment-739113376
@chris8099 Could you rebase code to include https://github.com/apache/kafka/commit/b9640a71c42ad117ed418209f87dc2962173469d?
----------------------------------------------------------------
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] [kafka] chris8099 commented on pull request #9694: MINOR:Modify aclsResources, discard Collection
Posted by GitBox <gi...@apache.org>.
chris8099 commented on pull request #9694:
URL: https://github.com/apache/kafka/pull/9694#issuecomment-738978021
@chia7712 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] [kafka] chia7712 commented on pull request #9694: MINOR: Change the input type of DescribeAclsResponse#aclsResources from Collection to Iterable to avoid creating useless collection
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9694:
URL: https://github.com/apache/kafka/pull/9694#issuecomment-740342002
@chris8099 Could you rebase code to include https://github.com/apache/kafka/commit/8db3b1a09af0bad274e07161336994610d616b35? Also, please fix the conflicting.
----------------------------------------------------------------
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] [kafka] chris8099 commented on pull request #9694: MINOR:Drop Collection and change to another method
Posted by GitBox <gi...@apache.org>.
chris8099 commented on pull request #9694:
URL: https://github.com/apache/kafka/pull/9694#issuecomment-739645972
@chia7712 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] [kafka] ijuma commented on a change in pull request #9694: MINOR: Change the input type of DescribeAclsResponse#aclsResources from Collection to Iterable to avoid creating useless collection
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9694:
URL: https://github.com/apache/kafka/pull/9694#discussion_r539394353
##########
File path: core/src/main/scala/kafka/server/KafkaApis.scala
##########
@@ -2509,12 +2509,10 @@ class KafkaApis(val requestChannel: RequestChannel,
.setThrottleTimeMs(requestThrottleMs)))
case Some(auth) =>
val filter = describeAclsRequest.filter
- val returnedAcls = new util.HashSet[AclBinding]()
- auth.acls(filter).forEach(returnedAcls.add)
Review comment:
Note that this means we no longer filter duplicates. We should make sure that doesn't cause issues.
----------------------------------------------------------------
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] [kafka] chia7712 commented on a change in pull request #9694: MINOR: Change the input type of DescribeAclsResponse#aclsResources from Collection to Iterable to avoid creating useless collection
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #9694:
URL: https://github.com/apache/kafka/pull/9694#discussion_r537254318
##########
File path: core/src/main/scala/kafka/server/KafkaApis.scala
##########
@@ -2509,12 +2509,10 @@ class KafkaApis(val requestChannel: RequestChannel,
.setThrottleTimeMs(requestThrottleMs)))
case Some(auth) =>
val filter = describeAclsRequest.filter
Review comment:
Could we avoid this local variable?
----------------------------------------------------------------
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] [kafka] chia7712 commented on a change in pull request #9694: MINOR: Change the input type of DescribeAclsResponse#aclsResources from Collection to Iterable to avoid creating useless collection
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #9694:
URL: https://github.com/apache/kafka/pull/9694#discussion_r539416849
##########
File path: core/src/main/scala/kafka/server/KafkaApis.scala
##########
@@ -2509,12 +2509,10 @@ class KafkaApis(val requestChannel: RequestChannel,
.setThrottleTimeMs(requestThrottleMs)))
case Some(auth) =>
val filter = describeAclsRequest.filter
- val returnedAcls = new util.HashSet[AclBinding]()
- auth.acls(filter).forEach(returnedAcls.add)
Review comment:
Good point! It seems to me the method ```aclsResources``` should de-duplicate as it tries to traverse all elements from input
----------------------------------------------------------------
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] [kafka] chia7712 commented on pull request #9694: MINOR:Drop Collection and change to another method
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9694:
URL: https://github.com/apache/kafka/pull/9694#issuecomment-739647110
@chris8099 Could you revise the title? It seems to me the title is not clear.
----------------------------------------------------------------
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