refactor #112: reorganize config CLI and fix help-mode crash#230
refactor #112: reorganize config CLI and fix help-mode crash#230tomschr merged 2 commits intoopenSUSE:mainfrom
Conversation
Coverage ReportFor commit 97a17ac 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_config/list.py 26 0 8 1 97.1%
+ 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_cli.py 110 3 16 3 95.2%
- 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_config/validate.py 21 2 12 3 78.8%
- 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%
--------------------------------------------------------------------------------
+ TOTAL 3047 67 732 30 97.0%
47 files skipped due to complete coverage. |
tomschr
left a comment
There was a problem hiding this comment.
Thanks for all your efforts! 👍
I realized that I didn't tell you an important aspect. I've described it below.
Have added some suggestions around some design questions.
There was a problem hiding this comment.
Thank you very much Sushant! Great work! 👍
I'm sorry that I found some issues that I wasn't aware before. Especially the validation part is tricky.
The validation of Docserv config works differently between old and new. With the new Portal schema, validation becomes actually easier.
As I'm still fixing the last bits on PR #197, I think we should remove the validation part for XML for the time being. Therefor it doesn't make sense to implement XML validation in this PR. It's also easier for you and you don't have to touch that twice.
For this reason, I've created issue #231 to separate these concerns. You can find all the details there. I also attached #231 as sub-issue.
Let me know what you think and sorry for the hassle.
tomschr
left a comment
There was a problem hiding this comment.
I just realized, we also need to adjust the documentation:
-
https://opensuse.github.io/docbuild/user/config/index.html
Mention the additional options and adjust the commands. See the files in
docs/source/user/config. -
https://opensuse.github.io/docbuild/user/validate.html
This needs to be removed as we will have a different subcommand in the future. See file
docs/source/user/validate.rst
Hi Toms, I've updated the PR as requested. I have removed all logic related to XML/Portal validation and moved it to a 'deferred' status. The validate command now focuses strictly on TOML health. I've also kept the core fixes for the JSON serialization bug and added the requested unit tests for the flattening utility. Let me know if this streamlined version is better. |
tomschr
left a comment
There was a problem hiding this comment.
Thank you very much, Sushant! Great work! 👍
I have only two issue:
- Could you try to revert the deletion of anything in regards to XML validation (code + test)? We need that in a later, follow-up PR.
Or do you see a better solution? - Lazy validation? The script doesn't show the help output but tries to validate it. Not sure if this could be caught, but that's a bit unfortunate. IMHO, the script should trigger validation when the user tries to show the help page.
Signed-off-by: sushant-suse <sushant.gaurav@suse.com>
tomschr
left a comment
There was a problem hiding this comment.
All fine now, but docs in regards to env/app aren't visible, right?
Signed-off-by: sushant-suse <sushant.gaurav@suse.com>
Hi Toms, added them back |
Related Issue #112
Key Changes
Lazy-Loading & Help Logic (
src/docbuild/cli/cmd_cli.py)Implemented a "Help Shield": The root
clifunction now detects if a help flag (--helpor-h) is present insys.argvbefore attempting to load any configuration files.Task-Oriented Config Subcommands
Merged Resources: Replaced the separate config application and config environment commands with unified config list and config validate commands. Added config list: Supports
--app,--env,andflags with an optional--portal--flatoutput mode for "git-style" dotted configuration views. Added config validate: Centralizes validation logic for both application and environment files in one place.Test Suite Stabilization
Metadata Logging: Updated
tests/cli/cmd_metadata/test_cmd_metadata.pyto useunittest.mock.patchon the module-level logger. This ensures tests passing by bypassing caplog capture issues in async environments.Help Verification: Added
tests/cli/test_help.pyto specifically verify thatload_app_configis never called when the help flag is passed.