Skip to content

Fix #191: Add producer-consumer model#194

Merged
tomschr merged 26 commits intomainfrom
toms/191-producer-consumer
Apr 13, 2026
Merged

Fix #191: Add producer-consumer model#194
tomschr merged 26 commits intomainfrom
toms/191-producer-consumer

Conversation

@tomschr
Copy link
Copy Markdown
Contributor

@tomschr tomschr commented Feb 24, 2026

This PR plays around with the producer-consumer model requested in issue #191. It maybe be integrated or it goes nowhere.

To test it, use these commands:

source devel/activate-aliases.sh
upython -m docbuild.utils.concurrency

Questions:

  • The current implementation uses TaskGroups, which is considered as the go-to way. Semaphores seems not always that reliable or appropriate for this use case.
    => We use TaskGroups
  • What do we want to "return" inside the consumer() loop?
    The consumer calls the worker function. Depending on if it's successful or not, we add either a Success object or a Failure object.
    I'm not sure if this is a bit too much and can be simplified.

    => We return the respective object and if something fails, a TaskFailedError exception.
  • I'm not sure about the naming: process_unordered. Possible better names:
    • execute_concurrently
    • run_in_parallel

@tomschr tomschr added the question Further information is requested label Feb 24, 2026
@tomschr tomschr marked this pull request as draft February 24, 2026 15:48
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 25, 2026

Coverage Report

For commit 691014b

Click to expand Coverage Report
  Name                                           Stmts   Miss Branch BrPart  Cover
  --------------------------------------------------------------------------------
+ src/docbuild/models/deliverable.py               180      1     22      0  99.5%
+ src/docbuild/cli/cmd_check/process.py             58      0     22      1  98.8%
+ src/docbuild/models/manifest.py                  111      1     12      1  98.4%
+ src/docbuild/utils/pidlock.py                     79      1     14      1  97.8%
+ src/docbuild/cli/cmd_cli.py                       96      1     10      2  97.2%
+ src/docbuild/cli/cmd_validate/process.py         178      5     52      4  96.1%
+ src/docbuild/cli/callback.py                      35      0     10      2  95.6%
+ src/docbuild/utils/concurrency.py                 69      3     18      1  95.4%
- src/docbuild/cli/cmd_config/__init__.py            9      1      0      0  88.9%
- src/docbuild/config/xml/stitch.py                 47      5     12      0  88.1%
- src/docbuild/cli/cmd_metadata/metaprocess.py     215     26     66     13  82.6%
- src/docbuild/cli/cmd_check/__init__.py            18      5      2      0  65.0%
- src/docbuild/cli/cmd_build/__init__.py            13      5      0      0  61.5%
- src/docbuild/cli/cmd_metadata/__init__.py         27     10      2      0  58.6%
- src/docbuild/cli/cmd_config/environment.py        11      6      2      0  38.5%
  --------------------------------------------------------------------------------
+ TOTAL                                           2992     70    706     25  96.9%
  
  46 files skipped due to complete coverage.

@tomschr tomschr requested a review from sushant-suse February 25, 2026 14:23
@sushant-suse
Copy link
Copy Markdown
Collaborator

This PR plays around with the producer-consumer model requested in issue #191. It maybe be integrated or it goes nowhere.

To test it, use these commands:

source devel/activate-aliases.sh
upython -m docbuild.utils.concurrency

Questions:

  • The current implementation uses TaskGroups, which is considered as the go-to way. Semaphores seems not always that reliable or appropriate for this use case.
    => We use TaskGroups

  • What do we want to "return" inside the consumer() loop?
    The consumer calls the worker function. Depending on if it's successful or not, we add either a Success object or a Failure object.
    I'm not sure if this is a bit too much and can be simplified.

    => We return the respective object and if something fails, a TaskFailedError exception.

  • I'm not sure about the naming: process_unordered. Possible better names:

    • execute_concurrently
    • run_in_parallel

Thanks for the awesome work Toms. It looks great. I have also added my suggestions.

Comment thread src/docbuild/utils/concurrency.py Outdated
Comment thread src/docbuild/utils/concurrency.py Outdated
Comment thread src/docbuild/utils/concurrency.py
Comment thread src/docbuild/utils/concurrency.py Outdated
@tomschr tomschr marked this pull request as ready for review March 2, 2026 09:30
@tomschr tomschr requested a review from sushant-suse March 2, 2026 09:31
Comment thread src/docbuild/utils/concurrency.py Outdated
Comment thread src/docbuild/utils/concurrency.py Outdated
@tomschr tomschr marked this pull request as draft March 18, 2026 06:52
tomschr and others added 20 commits March 24, 2026 13:04
* Rename function to `process_unordered`
* Remove Result, Success, and Failure
* For the result, just add the result from the worker or
  the exception.
to ensure backpressure propagates through the whole pipeline
The intention is to pass *worker_args and **worker_kwargs
to the worker_fn function.
We need to explicity tell the type checker that worker_fn is
expected to accept the arguments from P in addition to the
standard item of type T. We use typing.Concatenate.
@tomschr tomschr force-pushed the toms/191-producer-consumer branch from e1fb882 to fb6738e Compare March 24, 2026 12:04
@sushant-suse sushant-suse self-assigned this Apr 13, 2026
Copy link
Copy Markdown
Contributor Author

@tomschr tomschr left a comment

Choose a reason for hiding this comment

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

Ready to be squash-merged! 👍 🎉

@tomschr tomschr marked this pull request as ready for review April 13, 2026 14:05
@tomschr tomschr force-pushed the toms/191-producer-consumer branch from 77c6f79 to 691014b Compare April 13, 2026 14:12
@tomschr tomschr merged commit 8e463b0 into main Apr 13, 2026
9 checks passed
@tomschr tomschr deleted the toms/191-producer-consumer branch April 13, 2026 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants