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/08/02 05:34:25 UTC

[GitHub] [kafka] abbccdda commented on a change in pull request #9103: Add redirection for (Incremental)AlterConfig

abbccdda commented on a change in pull request #9103:
URL: https://github.com/apache/kafka/pull/9103#discussion_r462738230



##########
File path: clients/src/main/java/org/apache/kafka/clients/ClientRequest.java
##########
@@ -85,11 +89,12 @@ public ApiKeys apiKey() {
     public RequestHeader makeHeader(short version) {
         short requestApiKey = requestBuilder.apiKey().id;
         return new RequestHeader(
-            new RequestHeaderData().
-                setRequestApiKey(requestApiKey).
-                setRequestApiVersion(version).
-                setClientId(clientId).
-                setCorrelationId(correlationId),
+            new RequestHeaderData()

Review comment:
       You commented on the previous PR about the style here. The reasoning is that this is a more common style than having period at the end in our codebase.

##########
File path: clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java
##########
@@ -151,6 +147,20 @@ public void testDnsLookupFailure() {
         assertFalse(client.ready(new Node(1234, "badhost", 1234), time.milliseconds()));
     }
 
+    @Test
+    public void testIncludeInitialPrincipalNameAndClientIdInHeader() {

Review comment:
       This is the new test, the rest of changes in this file are just side cleanups.

##########
File path: clients/src/main/java/org/apache/kafka/common/requests/AlterConfigsRequest.java
##########
@@ -64,23 +64,11 @@ public String value() {
 
     public static class Builder extends AbstractRequest.Builder<AlterConfigsRequest> {
 
-        private final AlterConfigsRequestData data = new AlterConfigsRequestData();
+        private final AlterConfigsRequestData data;
 
-        public Builder(Map<ConfigResource, Config> configs, boolean validateOnly) {

Review comment:
       Moved to `AlterConfigsUtil`

##########
File path: clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
##########
@@ -2207,27 +2203,6 @@ void handleFailure(Throwable throwable) {
         return futures;
     }
 
-    private IncrementalAlterConfigsRequestData toIncrementalAlterConfigsRequestData(final Collection<ConfigResource> resources,

Review comment:
       Moved to `AlterConfigsUtil`




----------------------------------------------------------------
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