Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement KIP-204 : DeleteRecords API #969

Merged
merged 6 commits into from
Jan 29, 2024
Merged

Conversation

vmaurin
Copy link
Contributor

@vmaurin vmaurin commented Jan 25, 2024

When doing stream processing, it is convinient to use "transient" topic :

  • retention time is infinite
  • records get deleted when consumed

The java kafka streams client is using the deleteRecords of the admin client to perform this operation. It is lacking in aiokafka

The KIP reference https://cwiki.apache.org/confluence/display/KAFKA/KIP-204+%3A+Adding+records+deletion+operation+to+the+new+Admin+Client+API

refs #967

Changes

Fixes #967

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> (e.g. 588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the PR
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: Fix issue with non-ascii contents in doctest text files.

When doing stream processing, it is convinient to use "transient" topic
:
* retention time is infinite
* records get deleted when consumed

The java kafka streams client is using the deleteRecords of the admin
client to perform this operation. It is lacking in aiokafka

The KIP reference https://cwiki.apache.org/confluence/display/KAFKA/KIP-204+%3A+Adding+records+deletion+operation+to+the+new+Admin+Client+API

refs aio-libs#967
@vmaurin vmaurin marked this pull request as draft January 25, 2024 16:27
@ods ods changed the title Implement KIP-202 : DeleteRecords API Implement KIP-204 : DeleteRecords API Jan 29, 2024
aiokafka/admin/client.py Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jan 29, 2024

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (f8d0d15) 95.06% compared to head (1395718) 95.03%.

Files Patch % Lines
aiokafka/admin/client.py 75.86% 4 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #969      +/-   ##
==========================================
- Coverage   95.06%   95.03%   -0.03%     
==========================================
  Files         106      107       +1     
  Lines       16332    16408      +76     
  Branches     2610     2624      +14     
==========================================
+ Hits        15526    15594      +68     
- Misses        532      536       +4     
- Partials      274      278       +4     
Flag Coverage Δ
cext 91.74% <91.25%> (-0.02%) ⬇️
integration 94.68% <91.25%> (-0.03%) ⬇️
purepy 94.49% <91.25%> (-0.04%) ⬇️
unit 44.91% <53.75%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

aiokafka/admin/client.py Outdated Show resolved Hide resolved
aiokafka/protocol/admin.py Outdated Show resolved Hide resolved
Vincent Maurin and others added 3 commits January 29, 2024 17:39
TaggedFields doesn't seem to work properly at the moment. Maybe they
should be replaced by an implementation closer to the java client with
their "flexibleVersions"
@ods
Copy link
Collaborator

ods commented Jan 29, 2024

@vmaurin I think it's ready to merge, no objections?

@vmaurin
Copy link
Contributor Author

vmaurin commented Jan 29, 2024

@ods Sure, thank you for fixing my mistake 👍

@vmaurin vmaurin marked this pull request as ready for review January 29, 2024 19:01
@ods ods merged commit 82695b0 into aio-libs:master Jan 29, 2024
29 of 31 checks passed
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.

Add delete_records to the admin client
2 participants