fix(cli): surface YAML and AsyncAPI errors from docs serve - #2874
Conversation
|
Sorry for the long wait on this PR, and thank you for your patience. The overall direction looks good, and the improved validation diagnostics are useful. Before we merge it, could you please make the following adjustments:
The YAML parsing error handling and surfaced Pydantic validation errors are otherwise a useful improvement. Thanks for working on this. |
Addresses review on ag2ai#2874: - Show the quoting hint only for .yaml/.yml inputs. A numeric value in a JSON document is a real schema error, not a PyYAML coercion, so the hint would only mislead there. - Emit the hint at most once instead of once per AsyncAPI version. - Add a JSON regression test asserting the hint is absent. - Cover the exact ag2ai#2709 reproduction, including protocolVersion set to a tagged scalar. Signed-off-by: SarthakB11 <sarthak.bhardwaj21b@iiitg.ac.in>
|
All four are addressed: the hint now fires only for .yaml/.yml and prints once, a JSON test confirms it stays absent, and a new test covers the |
|
Can you resolve problem with tests? |
Addresses review on ag2ai#2874: - Show the quoting hint only for .yaml/.yml inputs. A numeric value in a JSON document is a real schema error, not a PyYAML coercion, so the hint would only mislead there. - Emit the hint at most once instead of once per AsyncAPI version. - Add a JSON regression test asserting the hint is absent. - Cover the exact ag2ai#2709 reproduction, including protocolVersion set to a tagged scalar. Signed-off-by: SarthakB11 <sarthak.bhardwaj21b@iiitg.ac.in>
733668a to
3d03e8b
Compare
|
Looked into the 3 failures: test_worker_id_extra_option.py, test_run_asgi.py::test_many_workers, test_logs.py::test_run_as_asgi_mp_with_log_level. All three are in the multi-worker CLI subprocess tests, a different subsystem from this PR's diff (faststream/_internal/cli/docs.py and its own test file only). All 6 tests in that area pass locally for me on this branch. Rebased onto latest main and pushed to get a clean CI run, that should confirm it either way. |
|
@Sarthakb1 problem with the tests is relevant, try to rebase ur branch |
Description
Refs #2709 (diagnostic portion).
Per @Sehat1137's recommendation on #2858 ("a smaller PR limited to that behavior, with deterministic parsing and regression tests"), this is the diagnostic-only redo: no new CLI options, no parser plugin, default
yaml.safe_loadpreserved. The pluggable-parser proposal from #2709 is intentionally deferred per maintainer feedback.faststream docs serve <asyncapi.yaml>previously collapsed three different failure modes into one opaqueSCHEMA_NOT_SUPPORTEDmessage:yaml.safe_loadran outside anytry, so the user either got an uncaught traceback or, more often, a downstream pydantic failure on the garbage that partial parsing produced.contextlib.suppress(ValidationError), so there was no indication of which schema version was tried or why it failed.protocolVersion: 3.2(parsed as afloat, rejected by AsyncAPI'sstringtype) gave no hint that the value just needed quoting. This is the exact repro from Bug:PyYAMLcannot correct parseasyncapi.yaml#2709.Fix
Edits only
_parse_and_serveinfaststream/_internal/cli/docs.py:yaml.safe_loadintry / except yaml.YAMLError, echo the parser exception, exit 1.suppress(ValidationError)with an explicit collector that records(version_label, ValidationError). When neither v3.0 nor v2.6 validates, printSCHEMA_NOT_SUPPORTEDfollowed by both per-version error blocks on stderr.type == "string_type"and anint | float | boolinput, append a one-line hint pointing at the location and suggesting YAML quoting (e.g.'3.2').Default parser is unchanged.
_parse_and_serve's public signature is untouched.dto.pyis untouched. No new modules, no parser registry, no--yaml-parserflag.Tests
Two regression tests in
tests/cli/test_asyncapi_docs.py:test_serve_asyncapi_yaml_unquoted_scalar_reports_hintmutates the existingyaml_asyncapi_docfixture toprotocolVersion: 3.2, asserts exit code 1 and the v3.0 / v2.6 error enumeration plus the quoting hint.test_serve_asyncapi_reports_yaml_parse_errorfeeds malformed YAML (foo: [unclosed) and asserts the parser exception is surfaced rather than swallowed.Both follow the existing
faststream_cli + generate_template + wait_for_stderrpattern in the same file.Type of change
Checklist
just lintshows no errors)just test-coveragejust static-analysis