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

Manually acknowledge take #192

Merged
merged 7 commits into from
Oct 26, 2021
Merged

Conversation

mdrago98
Copy link
Contributor

@mdrago98 mdrago98 commented Oct 6, 2021

Description

The following change adds a version of take which would allow the user to manually control when the batch is acknowledged. This would be useful for certain applications where we would need to reprocess the un acknowledged messages if the batch fails.

We also added a change to the setup prometheus sensors function which would check the name for naming consistencies with prometheus.

Refer to #189

@taybin
Copy link
Contributor

taybin commented Oct 12, 2021

I need to take a closer look at this, but a public facing API like this definitely needs one or more unittests.

@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2021

Codecov Report

Merging #192 (3c92bbc) into master (835f37e) will decrease coverage by 0.46%.
The diff coverage is 1.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #192      +/-   ##
==========================================
- Coverage   94.51%   94.04%   -0.47%     
==========================================
  Files         100      100              
  Lines       10631    10685      +54     
  Branches     1202     1207       +5     
==========================================
+ Hits        10048    10049       +1     
- Misses        514      567      +53     
  Partials       69       69              
Impacted Files Coverage Δ
faust/sensors/prometheus.py 92.85% <0.00%> (-2.94%) ⬇️
faust/streams.py 89.79% <2.08%> (-9.75%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 835f37e...3c92bbc. Read the comment docs.

Copy link
Collaborator

@patkivikram patkivikram left a comment

Choose a reason for hiding this comment

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

Could you add unit tests?

@patkivikram patkivikram merged commit 510051b into faust-streaming:master Oct 26, 2021
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