In C++ partitioned consumer, do not append partition index in subscription name #836
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
LGTM, but will cause the ppl using the old client to lose messages on the incorrect subscription name. |
That's correct, though the idea is to fix it before anyone starts using it |
|
retest this please |
saandrews
reviewed
Oct 18, 2017
saandrews
approved these changes
Oct 18, 2017
merlimat
force-pushed
the
fix-c++-partitioned-consumer
branch
from
99aa60e to
8e079d6
Compare
massakam
reviewed
Oct 19, 2017
| ASSERT_FALSE(res != 204 && res != 409); | ||
|
|
||
| Consumer partitionedConsumer; | ||
| Result result = client.subscribe(topicName, "subscription-A", consumer); |
There was a problem hiding this comment.
Is the variable name consumer (not partitionedConsumer) correct?
|
retest this please |
|
👍 |
massakam
approved these changes
Oct 23, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
As described in #827, the C++ partitioned consumer is appending the partition id to the subscription name, unlike the Java partitioned consumer. That breaks the assumptions made by many tools and APIs to associate the same subscription id across all the partitions (eg: unsubscribe a subscription from all partitions at once).
Modifications
Make C++ client to use the same convention as Java client in this case.