Check _shutdown flag before executing serial operations#10742
Open
bysiber wants to merge 1 commit intopython-poetry:mainfrom
Open
Check _shutdown flag before executing serial operations#10742bysiber wants to merge 1 commit intopython-poetry:mainfrom
bysiber wants to merge 1 commit intopython-poetry:mainfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsure executor stops executing serial operations once shutdown is signaled, and add a markdown file documenting a separate sdist suffix-handling bug and its intended fix. Sequence diagram for executor shutdown affecting serial operationssequenceDiagram
participant Executor
participant ParallelTasks
participant SerialOperation
Executor->>ParallelTasks: submit parallel operations
ParallelTasks-->>Executor: futures
Executor->>ParallelTasks: wait(tasks)
ParallelTasks-->>Executor: one task fails, sets Executor._shutdown = True
loop serial_operations
Executor->>Executor: check _shutdown
alt _shutdown is True
Executor-->>Executor: break out of loop
else _shutdown is False
Executor->>SerialOperation: _execute_operation(operation)
SerialOperation-->>Executor: result
end
end
alt shutdown after error or KeyboardInterrupt
Executor-->>Executor: skip remaining work and exit
end
Class diagram for Executor shutdown and serialization logicclassDiagram
class Executor {
bool _shutdown
_serialize(operations, priority_group)
_execute_operation(operation)
execute(operations)
}
class Operation {
int priority
bool is_parallel
bool is_serial
}
Executor "1" o-- "*" Operation : executes
class ParallelExecution {
wait(tasks)
}
Executor ..> ParallelExecution : uses for parallel operations
note for Executor "In _serialize, Executor now checks _shutdown before each serial operation and breaks the loop when _shutdown is True"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
pr_body_2.mdfile looks like PR description/supporting notes rather than source content; consider removing it from the repo or relocating it to a more appropriate place (e.g. the PR description or an issue) so it doesn’t ship with the package.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `pr_body_2.md` file looks like PR description/supporting notes rather than source content; consider removing it from the repo or relocating it to a more appropriate place (e.g. the PR description or an issue) so it doesn’t ship with the package.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
When a parallel task fails and sets _shutdown = True, the executor still unconditionally runs all serial operations (uninstalls and develop-mode installs) in the same priority group. wait() doesn't re-raise thread exceptions, so control falls through to the serial loop. The _shutdown check only happens after the try-except block. This can leave the environment in a worse state — packages get uninstalled after the installation was supposed to have stopped, creating a partial environment that's harder to recover from than just aborting early.
4b0dd0c to
b722ef9
Compare
radoering
reviewed
Feb 21, 2026
Member
radoering
left a comment
There was a problem hiding this comment.
Wouldn't it make sense to put this check at the beginning of _execute_operation so that waiting tasks are also aborted early?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In
Executor.execute(), when a parallel task fails and setsself._shutdown = True, the serial operations in the same priority group still execute unconditionally.concurrent.futures.wait()does not re-raise exceptions from worker threads — it waits for all futures to complete and returns. After it returns, the loop overserial_operationsruns every serial operation without checking_shutdown. The shutdown check only happens after thetry/exceptblock.This matters when parallel installs and serial uninstalls are in the same priority group. A failed compilation in a parallel install should stop the whole process, but instead all uninstalls still proceed, potentially leaving the environment in a worse state than before.
Fix
Check
self._shutdownbefore each serial operation:Summary by Sourcery
Guard serial installation operations with the executor shutdown flag and document a separate sdist archive filename handling issue and its fix proposal.
Bug Fixes:
Documentation: