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

improv: Make regen consume items more reliably #6799

Merged
merged 1 commit into from
Jun 16, 2021

Conversation

maybe-sybr
Copy link
Contributor

Description

This is a follow up to #6789 with some more significant changes to the regen
implementation to:

  • avoid re-iterating previously consumed elements in __getitem__()
  • mark the instance as done when a single lookahead stops iterating
    ** this fixes an edge case where a repeating underlying iterator could be duplicated if regen is not fully iterated
    ** element duplication from repeatable underlying iterables is not generally fixed since we still store the iterable not its iterator (IIUC)

@maybe-sybr
Copy link
Contributor Author

This is based on the diff currently up at #6789 - the substantive diff is just 08066a1

@codecov
Copy link

codecov bot commented Jun 3, 2021

Codecov Report

Merging #6799 (645c0ef) into master (5d72aee) will increase coverage by 0.00%.
The diff coverage is 92.30%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6799   +/-   ##
=======================================
  Coverage   70.73%   70.73%           
=======================================
  Files         138      138           
  Lines       16605    16615   +10     
  Branches     2094     2098    +4     
=======================================
+ Hits        11745    11753    +8     
- Misses       4663     4664    +1     
- Partials      197      198    +1     
Flag Coverage Δ
unittests 70.73% <92.30%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
celery/utils/functional.py 71.42% <92.30%> (+0.54%) ⬆️

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 5d72aee...645c0ef. Read the comment docs.

@lgtm-com
Copy link

lgtm-com bot commented Jun 3, 2021

This pull request introduces 1 alert when merging 08066a1 into 799f839 - view on LGTM.com

new alerts:

  • 1 for Non-exception in 'except' clause

This change introduces a helper method which abstracts the logic of
consuming items one by one in `regen` and also introduces a single
lookahead to ensure that the `__done` property gets set even if the
regen is not fully iterated, fixing an edge case where a repeatable
iterator would get doubled when used as a base for a `regen` instance.
@maybe-sybr maybe-sybr force-pushed the improv/regen-lazy-consumption branch from 08066a1 to 645c0ef Compare June 16, 2021 04:12
@maybe-sybr maybe-sybr marked this pull request as ready for review June 16, 2021 04:12
@lgtm-com
Copy link

lgtm-com bot commented Jun 16, 2021

This pull request introduces 1 alert and fixes 1 when merging 645c0ef into 5d72aee - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 1 for Module is imported with 'import' and 'import from'

@auvipy auvipy added this to the 5.1.x milestone Jun 16, 2021
@auvipy auvipy merged commit d667f1f into master Jun 16, 2021
@thedrow thedrow deleted the improv/regen-lazy-consumption branch June 16, 2021 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants