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/10/08 19:05:47 UTC
[GitHub] [kafka] efeg opened a new pull request #9397: KAFKA-10583: Add documentation on the thread-safety of KafkaAdminClient.
efeg opened a new pull request #9397:
URL: https://github.com/apache/kafka/pull/9397
Other than a Stack Overflow comment (see https://stackoverflow.com/a/61738065) by Colin Patrick McCabe (@cmccabe ) and a proposed design note on KIP-117 wiki, there is no source that verifies the thread-safety of `KafkaAdminClient`.
This patch updates JavaDoc of `KafkaAdminClient` to clarify its thread-safety.
----------------------------------------------------------------
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] tombentley commented on pull request #9397: KAFKA-10583: Add documentation on the thread-safety of KafkaAdminClient.
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9397:
URL: https://github.com/apache/kafka/pull/9397#issuecomment-706196779
@efeg the javadoc for `KafkaAdminClient` says "Users should not refer to this class directly", so just adding the thread-safety note to `KafkaAdminClient` probably isn't so helpful, we should add it to `Admin` too. Though since that's an interface probably some wording like "The instances returned from the {@code create} methods are guaranteed to be thread safe" would be better.
This is also an opportunity to improve the rather scant Javadoc for `Admin`. Compare it to the extensive Javadoc for `KafkaProducer` and `KafkaConsumer`.
----------------------------------------------------------------
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] tombentley commented on pull request #9397: KAFKA-10583: Add documentation on the thread-safety of KafkaAdminClient.
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9397:
URL: https://github.com/apache/kafka/pull/9397#issuecomment-708304114
@cmccabe @ijuma @hachikuji any chance we could merge this trivial javadoc change?
----------------------------------------------------------------
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] efeg commented on pull request #9397: KAFKA-10583: Add documentation on the thread-safety of KafkaAdminClient.
Posted by GitBox <gi...@apache.org>.
efeg commented on pull request #9397:
URL: https://github.com/apache/kafka/pull/9397#issuecomment-705765230
@cmccabe Do you think you might be able to take a look at this 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:
users@infra.apache.org
[GitHub] [kafka] tombentley commented on pull request #9397: KAFKA-10583: Add documentation on the thread-safety of KafkaAdminClient.
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9397:
URL: https://github.com/apache/kafka/pull/9397#issuecomment-711779114
@chia7712 `Tom Bentley <tb...@redhat.com>` is what's used in my commits.
----------------------------------------------------------------
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] efeg commented on pull request #9397: KAFKA-10583: Add documentation on the thread-safety of KafkaAdminClient.
Posted by GitBox <gi...@apache.org>.
efeg commented on pull request #9397:
URL: https://github.com/apache/kafka/pull/9397#issuecomment-705765230
@cmccabe Do you think you might be able to take a look at this 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:
users@infra.apache.org
[GitHub] [kafka] chia7712 merged pull request #9397: KAFKA-10583: Add documentation on the thread-safety of KafkaAdminClient.
Posted by GitBox <gi...@apache.org>.
chia7712 merged pull request #9397:
URL: https://github.com/apache/kafka/pull/9397
----------------------------------------------------------------
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] tombentley commented on pull request #9397: KAFKA-10583: Add documentation on the thread-safety of KafkaAdminClient.
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #9397:
URL: https://github.com/apache/kafka/pull/9397#issuecomment-711858164
@efeg do you want to have a go at writing a more thorough class Javadoc for `Admin`? If not I'll try to write something.
----------------------------------------------------------------
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 #9397: KAFKA-10583: Add documentation on the thread-safety of KafkaAdminClient.
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9397:
URL: https://github.com/apache/kafka/pull/9397#issuecomment-711789924
@efeg Thanks for the patch! merge it to trunk
----------------------------------------------------------------
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] efeg commented on pull request #9397: KAFKA-10583: Add documentation on the thread-safety of KafkaAdminClient.
Posted by GitBox <gi...@apache.org>.
efeg commented on pull request #9397:
URL: https://github.com/apache/kafka/pull/9397#issuecomment-708055229
@tombentley thanks for the feedback!
> @efeg the javadoc for `KafkaAdminClient` says "Users should not refer to this class directly", so just adding the thread-safety note to `KafkaAdminClient` probably isn't so helpful, we should add it to `Admin` too. Though since that's an interface probably some wording like "The instances returned from the {@code create} methods are guaranteed to be thread safe" would be better.
Makes sense -- updated the JavaDoc of `Admin`.
> This is also an opportunity to improve the rather scant Javadoc for `Admin`. Compare it to the extensive Javadoc for `KafkaProducer` and `KafkaConsumer`.
I agree that `Admin` JavaDoc would benefit from a proper revision to make it on par with the other client docs. That being said, to avoid scope creep, I would prefer to keep this PR about the thread-safety of Admin clients.
----------------------------------------------------------------
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] efeg commented on pull request #9397: KAFKA-10583: Add documentation on the thread-safety of KafkaAdminClient.
Posted by GitBox <gi...@apache.org>.
efeg commented on pull request #9397:
URL: https://github.com/apache/kafka/pull/9397#issuecomment-712272753
> @efeg do you want to have a go at writing a more thorough class Javadoc for Admin? If not I'll try to write something.
@tombentley Thanks for your help with this PR!
Please feel free to create the PR for a more thorough `Admin` Javadoc when you have a chance.
----------------------------------------------------------------
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 #9397: KAFKA-10583: Add documentation on the thread-safety of KafkaAdminClient.
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9397:
URL: https://github.com/apache/kafka/pull/9397#issuecomment-711208586
@tombentley I'm going to merge this PR but I failed to find out your email (used by reviewers list). The email in your PRs (for example https://github.com/apache/kafka/commit/775a08876a636cc9f4b6cfd89e305b8becf44670.patch) is ```tombentley@users.noreply.github.com``` and it is absolutely not your true email :)
----------------------------------------------------------------
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