Skip to content

feat #189: introduce max_workers key in app config#201

Merged
sushant-suse merged 3 commits intoopenSUSE:mainfrom
sushant-suse:issue#189-introduce-max_workers
Mar 16, 2026
Merged

feat #189: introduce max_workers key in app config#201
sushant-suse merged 3 commits intoopenSUSE:mainfrom
sushant-suse:issue#189-introduce-max_workers

Conversation

@sushant-suse
Copy link
Copy Markdown
Collaborator

Related Issue #189

This PR introduces a new max_workers key to the application configuration to manage concurrency levels dynamically.

Changes:

  • Model Update: Added max_workers to AppConfig in src/docbuild/models/config/app.py.
  • Resolution Logic: Implemented a field_validator that resolves:
    • all -> Total system CPU count.
    • half / all2 -> 50% of system CPU count (minimum 1).
    • int -> Strict worker count (minimum 1).
  • Defaults: Set the default value to half in src/docbuild/cli/defaults.py.

Validation:

  • Coverage: Increased project coverage (verified at 96.8% total).
  • Unit Tests: Added tests/models/test_config_workers.py and test_app_config.py to verify keyword resolution and boundary conditions.
  • Manual Test: Verified on local hardware (MacBook Pro) that "half" correctly resolves to 4 for an 8-core system.

Signed-off-by: sushant-suse <sushant.gaurav@suse.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 15, 2026

Coverage Report

For commit 723ed58

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/cli/cmd_cli.py                       93      1      8      1  98.0%
+ src/docbuild/utils/pidlock.py                     79      1     14      1  97.8%
+ 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/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                                           2920     67    686     23  97.0%
  
  46 files skipped due to complete coverage.

Copy link
Copy Markdown
Contributor

@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.

Thanks Sushant! The implementation looks fine. 👍 I would suggest some changes in the test cases. I've sketched a possible idea.

Comment thread tests/models/test_app_config.py Outdated
Comment thread tests/models/test_config_workers.py Outdated
Comment thread src/docbuild/models/config/app.py Outdated
Comment thread tests/models/test_config_workers.py Outdated
Comment thread tests/models/test_config_workers.py Outdated
Comment thread src/docbuild/models/config/app.py
…r max_workers

Signed-off-by: sushant-suse <sushant.gaurav@suse.com>
@sushant-suse
Copy link
Copy Markdown
Collaborator Author

Hi Toms, I've updated the PR based on your feedback. Here is a summary of the changes:

1. src/docbuild/models/config/app.py

  • Swapped the if/elif chain for a keyword_map using lambdas as suggested. This makes the worker resolution much more extensible.
  • Removed the redundant isinstance(v, int) check, keeping only the v < 1 safety check at the end.

2. tests/models/test_config_workers.py

  • Implemented the mock_cpu_count fixture to replace live os.cpu_count() calls.
  • Added None to the fixture parameters to verify that the logic correctly defaults to 1 when CPU count is indeterminate.
  • Split the keyword resolution into separate test cases (all, half, all2) for clearer failure reporting.

3. Cleanup

  • Deleted tests/models/test_app_config.py. This file was redundant now that the logic is fully covered by the deterministic tests in test_config_workers.py.

Copy link
Copy Markdown
Contributor

@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.

Great! I have only two minor things, otherwise good job! 👍

Comment thread tests/models/test_config_workers.py Outdated
Comment thread tests/models/test_config_workers.py Outdated
Signed-off-by: sushant-suse <sushant.gaurav@suse.com>
@sushant-suse sushant-suse merged commit 18f1128 into openSUSE:main Mar 16, 2026
9 checks passed
@sushant-suse sushant-suse deleted the issue#189-introduce-max_workers branch March 16, 2026 16:18
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