-
Notifications
You must be signed in to change notification settings - Fork 14.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KAFKA-18399 Remove ZooKeeper from KafkaApis (4/N): CREATE_TOPICS, DELETE_TOPICS, CREATE_PARTITIONS #18433
base: trunk
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TaiJuWu thanks for this patch.
@@ -212,8 +208,8 @@ class KafkaApis(val requestChannel: RequestChannel, | |||
case ApiKeys.LIST_GROUPS => handleListGroupsRequest(request).exceptionally(handleError) | |||
case ApiKeys.SASL_HANDSHAKE => handleSaslHandshakeRequest(request) | |||
case ApiKeys.API_VERSIONS => handleApiVersionsRequest(request) | |||
case ApiKeys.CREATE_TOPICS => maybeForwardToController(request, handleCreateTopicsRequest) | |||
case ApiKeys.DELETE_TOPICS => maybeForwardToController(request, handleDeleteTopicsRequest) | |||
case ApiKeys.CREATE_TOPICS => maybeForwardToController(request, null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should use KafkaApis.shouldAlwaysForward
to replace null
case ApiKeys.CREATE_TOPICS => maybeForwardToController(request, handleCreateTopicsRequest) | ||
case ApiKeys.DELETE_TOPICS => maybeForwardToController(request, handleDeleteTopicsRequest) | ||
case ApiKeys.CREATE_TOPICS => maybeForwardToController(request, null) | ||
case ApiKeys.DELETE_TOPICS => maybeForwardToController(request, null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
@@ -230,7 +226,7 @@ class KafkaApis(val requestChannel: RequestChannel, | |||
case ApiKeys.ALTER_REPLICA_LOG_DIRS => handleAlterReplicaLogDirsRequest(request) | |||
case ApiKeys.DESCRIBE_LOG_DIRS => handleDescribeLogDirsRequest(request) | |||
case ApiKeys.SASL_AUTHENTICATE => handleSaslAuthenticateRequest(request) | |||
case ApiKeys.CREATE_PARTITIONS => maybeForwardToController(request, handleCreatePartitionsRequest) | |||
case ApiKeys.CREATE_PARTITIONS => maybeForwardToController(request, null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All comments are address, thanks!
This reverts commit 6db17a2.
*More detailed description of your change
Delete follow KafkaApi handler
In Kraft mode, all requests are forwarded to controller and all authorization is not work so I delete some tests.
Forwarding test already cover by
Committer Checklist (excluded from commit message)