feat: Implement pretty-printing for Pydantic validation errors#182
feat: Implement pretty-printing for Pydantic validation errors#182sushant-suse merged 3 commits intoopenSUSE:mainfrom
Conversation
…errors Signed-off-by: sushant-suse <[email protected]>
Coverage ReportFor commit bc1085b Click to expand Coverage Report Name Stmts Miss Branch BrPart Cover
--------------------------------------------------------------------------------
+ src/docbuild/cli/cmd_check/process.py 58 0 22 1 98.8%
+ 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/config/xml/stitch.py 48 1 12 1 96.7%
+ 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_metadata/metaprocess.py 168 10 42 7 91.9%
- src/docbuild/cli/cmd_config/__init__.py 9 1 0 0 88.9%
- 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 2790 45 642 17 98.0%
48 files skipped due to complete coverage. |
tomschr
left a comment
There was a problem hiding this comment.
This is brilliant! 😍 It looks awesome!
One question in regards to output. For example, when the IP address is invalid, I get two errors:
(1) In 'server.host.function-plain[_validate()]':
value is not a valid IPv4 or IPv6 address
See: https://opensuse.github.io/docbuild/latest/errors/ip_any_address.html
(2) In 'server.host.constrained-str':
String should match pattern '^(?:[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?\.)+[a-zA-Z]{2,63}$'
See: https://opensuse.github.io/docbuild/latest/errors/string_pattern_mismatch.html
This may be technically correct. I guess, we can't avoid that, right?
That’s a great catch Toms! This happens because You're right that we can't easily avoid the double error without complex custom validators, but I can definitely improve the 'In' location string. Right now, it's showing internal Pydantic noise like I have updated the formatter to 'clean' the path so it just shows |
…atting Signed-off-by: sushant-suse <[email protected]>
Ahh, thanks for the explanation! I don't think we can't really avoid it. I was just curious. 😃 |
There was a problem hiding this comment.
That's a great progress! 👍 Apart from the minor suggestions below, I have only two final main points:
- The coverage for
cmd_cli.pyis ~82%. Could you raise it to >90%? - IMHO, the Pydantic validation errors would be a great addition to
docs/sources/user/config.rst, sectionValidationg the Configuration. You can remove the TODO then. 😉
Thank you so much!
…dge-case tests. Signed-off-by: sushant-suse <[email protected]>
Hi @tomschr, Thank you for the final suggestions! I have addressed both of your main points in the latest push:
The CI is green on macOS, and I’ve verified the formatting manually as well. Ready for your final look and approval! |
tomschr
left a comment
There was a problem hiding this comment.
That's brilliant! 😍 Great addition, thank you so much!
🚢 it!
Related Issue #158
Files Changed
src/docbuild/utils/errors.py:format_pydantic_error.-vvto see the full list.Fieldtitles and descriptions from the Pydantic models to show the user "what was expected".extra_forbidden,bool_parsing).src/docbuild/cli/cmd_cli.py:AppConfigandEnvConfigvalidation blocks to catchValidationError.None.tests/utils/test_errors.py:tests/cli/test_cmd_cli.py:Verification Results
ruff checkandpytest.