Skip to content

Deduplicate consecutive None items in SkipRepeatsQueue - #1348

Merged
BoboTiG merged 2 commits into
gorakhargosh:masterfrom
galafis:fix-queue-none-deduplication
Oct 25, 2026
Merged

BoboTiG merged 2 commits into
gorakhargosh:masterfrom
galafis:fix-queue-none-deduplication

Conversation

@galafis

@galafis galafis commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

SkipRepeatsQueue uses None to represent the absence of a previous item. As a result, q.put(None); q.put(None) queues both items and counts two unfinished tasks, although None is an immutable/hashable value and the queue promises to omit consecutive equal items.

Use a private sentinel for the empty state. Preserve the first-item short circuit for event objects, and keep delegation to queue.Queue and the existing producer lock. The regression checks queue size, task accounting, get/task_done/join, and allowing the same item again after the queue becomes empty. Parametrized controls cover zero, False and an empty string.

Validation on Windows/Python 3.12.14: before, the None regression fails and 54 queue controls pass; after, all 55 queue tests pass. An expanded run of queue, delayed-queue, observer and event tests reports 94 passed, with two existing unknown flaky-marker warnings because that optional plugin was not loaded. Ruff 0.17.2 checks/formatting, Mypy 2.4.0 for the changed source file and git diff --check pass. This is not the complete platform observer suite or eventlet/gevent compatibility validation.

@galafis

galafis commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

The remaining Windows 3.12 failure is tests/test_emitter.py::test_file_lifecyle: after touch, the native emitter reports FileAttribEvent where the test expects FileModifiedEvent. This also appeared once during local checks.

Ten subsequent alternating baseline/PR pairs, each in a fresh pytest process with the imported source verified, passed on Windows. All 55 queue tests also passed. These results do not establish whether the intermittent event classification is related to this patch, so no emitter or assertion changes have been added to the queue fix.

Could a maintainer rerun just the failed Windows 3.12 job?

@BoboTiG

BoboTiG commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Can you add a changelog line? In the commit, add "[skip ci]" so that the CI will be skipped.

@galafis

galafis commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Added the changelog entry under 7.0.0-dev → Other Changes in 6be8cee, with [skip ci] in the commit message as requested. Thanks!

@BoboTiG
BoboTiG merged commit af5a3db into gorakhargosh:master Oct 25, 2026
@BoboTiG

BoboTiG commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

Thanks @galafis 🥂

@BoboTiG

BoboTiG commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

@watchdog-bot please add @galafis for code

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