Skip to content

[fix][io] KCA: handle kafka sources that use commitRecord#176

Merged
dlg99 merged 1 commit intodatastax:2.10_dsfrom
dlg99:kca-commitrecord_ds
Apr 18, 2023
Merged

[fix][io] KCA: handle kafka sources that use commitRecord#176
dlg99 merged 1 commit intodatastax:2.10_dsfrom
dlg99:kca-commitrecord_ds

Conversation

@dlg99
Copy link
Collaborator

@dlg99 dlg99 commented Apr 15, 2023

Motivation

Support Kafka sources that use commitRecord() and do not implement commit()
Upstream PR: apache#20121

Modifications

added calls to commitRecord() in ack/fail

Verifying this change

  • Make sure that the change passes the CI checks.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@dlg99 dlg99 marked this pull request as draft April 15, 2023 00:53
Copy link

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

I agree with this approach.

Why do we need to add a configuration flag?

@dlg99 dlg99 force-pushed the kca-commitrecord_ds branch from 71fb2a9 to a0ff03e Compare April 17, 2023 20:51
@dlg99 dlg99 marked this pull request as ready for review April 17, 2023 21:59
@dlg99
Copy link
Collaborator Author

dlg99 commented Apr 17, 2023

@eolivelli I updated this a little. can't use commitRecord on fail, and removed configs.

@dlg99 dlg99 changed the title KCA: handle kafka sources that use commitRecord [fix][io] KCA: handle kafka sources that use commitRecord Apr 17, 2023
@dlg99 dlg99 merged commit b47d8c6 into datastax:2.10_ds Apr 18, 2023
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.

2 participants