Skip to content

KAFKA-2892: Consumer Docs Use Wrong Method#81

Closed
Jesse Anderson (eljefe6a) wants to merge 2 commits into
confluentinc:trunkfrom
eljefe6a:trunk
Closed

KAFKA-2892: Consumer Docs Use Wrong Method#81
Jesse Anderson (eljefe6a) wants to merge 2 commits into
confluentinc:trunkfrom
eljefe6a:trunk

Conversation

@eljefe6a

Copy link
Copy Markdown

The KafkaConsumer docs use a non-existent method for assigning partitions (consumer.assign).

The JavaDocs show as:

String topic = "foo";
TopicPartition partition0 = new TopicPartition(topic, 0);
TopicPartition partition1 = new TopicPartition(topic, 1);
consumer.assign(partition0);
consumer.assign(partition1);

Should be:

String topic = "foo";
TopicPartition partition0 = new TopicPartition(topic, 0);
TopicPartition partition1 = new TopicPartition(topic, 1);
consumer.assign(Arrays.asList(partition0));
consumer.assign(Arrays.asList(partition1));

@ewencp

Copy link
Copy Markdown

Good catch. assign doesn't actually allow incremental assignment though:

Manually assign a list of partition to this consumer. This interface does not allow for incremental assignment and will replace the previous assignment (if there is one).

It'd need to be consumer.assign(Arrays.asList(partition0, partition1)).

@eljefe6a

Copy link
Copy Markdown
Author

Ewen Cheslack-Postava (@ewencp) Updated.

@ewencp

Copy link
Copy Markdown

LGTM

@gwenshap

Copy link
Copy Markdown

LGTM, but I'm wondering if Jesse Anderson (@eljefe6a) didn't mean to submit this PR to Apache Kafka and not the Confluent copy?

@eljefe6a

Copy link
Copy Markdown
Author

Gwen Shapira (@gwenshap) That's correct. I just found out that you can't fork repos with different sources.

@gwenshap

Copy link
Copy Markdown

ah, so because you already forked confluent's fork you can't fork apache?

@eljefe6a

Copy link
Copy Markdown
Author

@gwenshap

Copy link
Copy Markdown

so, afaik, all of us here do "apache first" flow - we have a fork of apache kafka, we do PRs there and we update confluent kafka from apache.

That said, if you don't want to go to the trouble of creating a new fork and re-doing your work, we can probably merge it here and do a PR to apache Kafka from here.

@ijuma

Copy link
Copy Markdown
Member

This was submitted to the Apache repo and merged some time ago.

José Armando García Sancio (jsancio) pushed a commit that referenced this pull request Sep 13, 2019
Jenkins does not use ducker to run system tests, so include the public key file creation as part of the test itself rather than in the Docker image.
Andrew Egelhofer (andrewegel) pushed a commit that referenced this pull request Nov 16, 2021
List of commits from kafka-broker-plugins:

* Include all transitive dependencies in assembly (#71)
* Add OAuth LoginCallbackHandler for the Kafka server (#72)
* CPKAFKA-1593: Multi-tenant authorizer for CCloud (#66)
* CPKAFKA-1593: Integration tests for multi-tenant authorizer for CCloud (#67)
* CAAS-1960: Implement Phase 3.1 Metrics (#76)
* CAAS-1960: Add authenticated connections and authentication rate metrics per tenant (#77)
* Bump Confluent to 5.2.0-SNAPSHOT, Kafka to 2.2.0-SNAPSHOT
* Topic config constraints (#70)
* Remove redundant import in MultiTenantRequestContext (#79)
* 4.1.x to 5.0.x (#78)
* 5.0.x to 5.1.x (#82)
* CPKAFKA-1408: Add tenant throughput and request quota plugin (#68)
* Improve topic policy checks (#81)
* CPKAFKA-1410: Partition assignor to balance partitions at tenant level (#73)
* CPKAFKA-1694: Add partition throughput percentiles for monitoring (#84)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants