Skip to content

Comments

[feat] Support partitioned topic reader.#154

Merged
BewareMyPower merged 2 commits intoapache:mainfrom
shibd:mu_reader
Feb 7, 2023
Merged

[feat] Support partitioned topic reader.#154
BewareMyPower merged 2 commits intoapache:mainfrom
shibd:mu_reader

Conversation

@shibd
Copy link
Member

@shibd shibd commented Dec 23, 2022

Motivation

In the current implementation, Reader does not yet support partitioned topics.

Modifications

  • MultiTopicConsumer support hasMessageAvailable and acknowledgeCumulativeAsync method.
  • Reader support partitioned topic.

Verifying this change

  • Add all tests in ReaderTest to multi-partition topic scenarios.
  • Add ConsumerTest.testAcknowledgeCumulativeWithPartition unit test to cover MultiTopicConsumer.acknowledgeCumulativeAsync method.

Documentation

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@BewareMyPower
Copy link
Contributor

@shibd Could you resolve the conflicts and address the comments?

@shibd shibd requested a review from BewareMyPower January 18, 2023 01:51
@shibd shibd self-assigned this Jan 19, 2023
@shibd shibd added the enhancement New feature or request label Jan 19, 2023
@shibd shibd added this to the 3.2.0 milestone Jan 19, 2023
@shibd
Copy link
Member Author

shibd commented Feb 6, 2023

@BewareMyPower @RobertIndie PTAL, Because it blocked TableView pr.

Copy link
Contributor

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM. Just need to remove some const qualifiers on value parameters.

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants