Add --allow-remote-refs to disable HTTP fetching of $ref by default#3051
Merged
Conversation
Remote $ref fetching over HTTP/HTTPS is now disabled by default. When a $ref resolves to an HTTP(S) URL and --allow-remote-refs is not set, a clear error message is shown instead of silently fetching remote content. file:// URLs are still allowed without the flag since they are local. --url input implicitly enables --allow-remote-refs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a $ref points to a local file that doesn't exist, the error now clearly states "$ref file not found: <path>" instead of raising a raw FileNotFoundError. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Check status codes and Content-Type headers when fetching remote $ref targets over HTTP. Returns clear error messages for HTTP errors (4xx/5xx) and unexpected HTML responses instead of cryptic YAML parse errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an opt-in Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI
participant Config
participant Parser
participant HTTP
participant FileSystem
User->>CLI: invoke generator (maybe with --allow-remote-refs)
CLI->>Config: set allow_remote_refs
Config->>Parser: init parser(allow_remote_refs)
Parser->>Parser: encounter $ref
alt $ref is remote URL
Parser->>Parser: check allow_remote_refs
alt enabled
Parser->>HTTP: fetch URL
HTTP-->>HTTP: validate status & Content-Type
alt error (HTTP >=400 or HTML)
HTTP-->>Parser: raise SchemaFetchError
Parser-->>User: surface error
else valid content
HTTP-->>Parser: return body
Parser->>Parser: parse returned schema
end
else disabled
Parser-->>User: raise Error instructing to enable remote refs
end
else local path
Parser->>FileSystem: read file
alt not found
FileSystem-->>Parser: file not found
Parser-->>User: raise Error with attempted path
else found
FileSystem-->>Parser: return content
Parser->>Parser: parse local schema
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Merging this PR will not alter performance
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3051 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 86 87 +1
Lines 18031 18165 +134
Branches 2112 2081 -31
==========================================
+ Hits 18031 18165 +134
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The module-level import already covers these usages. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CLI reference docs will be auto-generated by CI from this marker. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
butvinm
force-pushed
the
fix/allow-remote-refs
branch
from
3afe40b to
88c5c74
Compare
Remove isinstance/hasattr guards from http.get_body() and instead
fix all test mocks to set status_code=200 and headers={}, matching
real httpx.Response behavior.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add SchemaFetchError(Error) for HTTP error status codes and unexpected HTML responses. This ensures errors are caught by the existing `except Error` handler in __main__.py and shown as clean messages instead of unhandled tracebacks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
tests/test_http.py (1)
17-60: Add one transport-error regression test.Great coverage on status/content-type checks. Please also cover
httpx.gettransport failures (e.g., timeout/connect error) to ensure they’re surfaced asSchemaFetchError.✅ Suggested additional test
+def test_get_body_raises_on_httpx_transport_error(mocker: MockerFixture) -> None: + """Test that transport-level httpx errors are wrapped consistently.""" + import httpx + + request = httpx.Request("GET", "https://example.com/schema.json") + mocker.patch("httpx.get", side_effect=httpx.ConnectError("boom", request=request)) + + with pytest.raises(SchemaFetchError, match="fetching"): + get_body("https://example.com/schema.json")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_http.py` around lines 17 - 60, Add a regression test that mocks httpx.get to raise a transport error and verifies get_body raises SchemaFetchError: create a new test (e.g., test_get_body_raises_on_transport_error) using MockerFixture to patch "httpx.get" to raise an httpx.TransportError (or httpx.ConnectTimeout) when called, then call get_body("https://example.com/schema.json") and assert it raises SchemaFetchError (matching an appropriate message); reference the existing tests and the get_body and SchemaFetchError symbols so the new test follows the same style and assertions as test_get_body_raises_on_http_error and test_get_body_succeeds_with_json_response.tests/parser/test_jsonschema.py (1)
58-59: Use concreteContent-Typevalues in the mocked success responses.The parser now validates response metadata before parsing remote schemas, but
headers = {}means these tests don't actually pin the allowed JSON/YAML media types. Setting representativeContent-Typeheaders here would make the success-path coverage match the contract introduced by this PR.Also applies to: 101-102, 141-142, 174-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/parser/test_jsonschema.py` around lines 58 - 59, The mocked HTTP responses in tests use mock_get.return_value.headers = {} so they don't assert the parser's Content-Type validation; update the tests to set concrete Content-Type header values on mock_get.return_value.headers (e.g., 'Content-Type': 'application/json' for JSON-schema success cases and 'Content-Type': 'application/x-yaml' or 'text/yaml' for YAML cases) at the occurrences where mock_get.return_value.status_code and headers are set (the mock_get.return_value.headers assignments around the blocks at lines referenced in the review: 58-59, 101-102, 141-142, and 174-175) so the success-path tests exercise the media-type checks used by the parser.tests/main/jsonschema/test_main_jsonschema.py (1)
816-821: Consider a shared mock-response helper for the new HTTP validation contract.These tests now repeat the same
status_code/headersscaffolding in many places. A tiny factory/helper would make future response-validation changes easier to update consistently.Also applies to: 857-862, 892-893, 911-912, 942-947, 2116-2117, 2260-2261, 7199-7200, 8953-8954
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/main/jsonschema/test_main_jsonschema.py` around lines 816 - 821, Tests repeat manual construction of mock responses (setting status_code, headers, text) which is error-prone; introduce a small factory function (e.g., make_mock_response or build_response_mock) used by tests in tests/main/jsonschema/test_main_jsonschema.py to create configured mock responses. The helper should accept common params (status_code, headers, text, json/data) and return a mock object with those attributes so you can replace the repeated blocks that set root_id_response and person_response (and the other occurrences listed) with calls to make_mock_response(status_code=200, headers={}, text="dummy") to centralize scaffolding and simplify future changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/datamodel_code_generator/http.py`:
- Around line 40-57: Network/transport errors from httpx.get can leak raw httpx
exceptions and Content-Type checks are case-sensitive; wrap the HTTP call in a
try/except that catches httpx.RequestError (and other httpx transport
exceptions) and re-raise a SchemaFetchError with the underlying error message,
and after receiving the response normalize the content type (e.g., content_type
= response.headers.get("content-type", "").lower()) before checking for
"text/html" (or matching JSON/YAML substrings) so comparisons are
case-insensitive; reference the httpx.get call, the response variable, the
content_type variable, and SchemaFetchError when making these changes.
In `@tests/main/jsonschema/test_main_jsonschema.py`:
- Around line 1010-1031: Update the assertion in
test_main_missing_local_ref_error_message so the stderr check verifies the
actual attempted ref path is included (e.g., "nonexistent.json" or the resolved
tmp_path string) instead of only the generic prefix; modify the call to
run_main_and_assert (or its expected_stderr_contains argument) to assert the
error message contains the specific ref filename "nonexistent.json" (or tmp_path
/ "nonexistent.json") so the test ensures the code surfaces which local $ref
failed.
- Around line 984-1007: The test test_main_remote_ref_blocked_by_default should
explicitly assert that no HTTP fetch occurs: patch httpx.get (or the HTTP client
used) at the start of the test, run run_main_and_assert with the same inputs,
and after the call assert the patched httpx.get was not called; update the test
function test_main_remote_ref_blocked_by_default to use a mock/monkeypatch for
httpx.get and assert no calls to it so the test guarantees no network request is
made even if stderr checking changes.
---
Nitpick comments:
In `@tests/main/jsonschema/test_main_jsonschema.py`:
- Around line 816-821: Tests repeat manual construction of mock responses
(setting status_code, headers, text) which is error-prone; introduce a small
factory function (e.g., make_mock_response or build_response_mock) used by tests
in tests/main/jsonschema/test_main_jsonschema.py to create configured mock
responses. The helper should accept common params (status_code, headers, text,
json/data) and return a mock object with those attributes so you can replace the
repeated blocks that set root_id_response and person_response (and the other
occurrences listed) with calls to make_mock_response(status_code=200,
headers={}, text="dummy") to centralize scaffolding and simplify future changes.
In `@tests/parser/test_jsonschema.py`:
- Around line 58-59: The mocked HTTP responses in tests use
mock_get.return_value.headers = {} so they don't assert the parser's
Content-Type validation; update the tests to set concrete Content-Type header
values on mock_get.return_value.headers (e.g., 'Content-Type':
'application/json' for JSON-schema success cases and 'Content-Type':
'application/x-yaml' or 'text/yaml' for YAML cases) at the occurrences where
mock_get.return_value.status_code and headers are set (the
mock_get.return_value.headers assignments around the blocks at lines referenced
in the review: 58-59, 101-102, 141-142, and 174-175) so the success-path tests
exercise the media-type checks used by the parser.
In `@tests/test_http.py`:
- Around line 17-60: Add a regression test that mocks httpx.get to raise a
transport error and verifies get_body raises SchemaFetchError: create a new test
(e.g., test_get_body_raises_on_transport_error) using MockerFixture to patch
"httpx.get" to raise an httpx.TransportError (or httpx.ConnectTimeout) when
called, then call get_body("https://example.com/schema.json") and assert it
raises SchemaFetchError (matching an appropriate message); reference the
existing tests and the get_body and SchemaFetchError symbols so the new test
follows the same style and assertions as test_get_body_raises_on_http_error and
test_get_body_succeeds_with_json_response.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 406afe18-7bde-422f-9922-8e134b30404a
📒 Files selected for processing (19)
src/datamodel_code_generator/__init__.pysrc/datamodel_code_generator/__main__.pysrc/datamodel_code_generator/_types/generate_config_dict.pysrc/datamodel_code_generator/_types/parser_config_dicts.pysrc/datamodel_code_generator/arguments.pysrc/datamodel_code_generator/cli_options.pysrc/datamodel_code_generator/config.pysrc/datamodel_code_generator/http.pysrc/datamodel_code_generator/parser/base.pysrc/datamodel_code_generator/parser/jsonschema.pytests/data/expected/main/input_model/config_class.pytests/main/jsonschema/test_main_jsonschema.pytests/main/openapi/test_main_openapi.pytests/main/test_main_json.pytests/main/test_public_api_signature_baseline.pytests/parser/test_jsonschema.pytests/parser/test_openapi.pytests/test_http.pytests/test_main_kr.py
Catch httpx transport exceptions (DNS, timeout, connection errors) and wrap them in SchemaFetchError. Normalize Content-Type to lowercase before checking for text/html. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/datamodel_code_generator/http.py (1)
49-51:⚠️ Potential issue | 🟠 MajorNarrow the exception handler to
httpx.RequestErroronly.Line 49 currently catches
Exception, which can hide programming errors and unexpected failures. Since onlyhttpx.get()is called in the try block, catchhttpx.RequestErrorinstead to handle transport/network errors specifically while allowing other exceptions to surface.Proposed change
- except Exception as e: + except httpx.RequestError as e: msg = f"Failed to fetch {url}: {e}" raise SchemaFetchError(msg) from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datamodel_code_generator/http.py` around lines 49 - 51, Narrow the broad exception handler around the httpx.get() call so it only catches transport/network errors: replace "except Exception as e" with "except httpx.RequestError as e" (the block that builds msg = f'Failed to fetch {url}: {e}' and raises SchemaFetchError(msg) from e) so other exceptions bubble up while network/request errors are wrapped in SchemaFetchError.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/datamodel_code_generator/http.py`:
- Around line 49-51: Narrow the broad exception handler around the httpx.get()
call so it only catches transport/network errors: replace "except Exception as
e" with "except httpx.RequestError as e" (the block that builds msg = f'Failed
to fetch {url}: {e}' and raises SchemaFetchError(msg) from e) so other
exceptions bubble up while network/request errors are wrapped in
SchemaFetchError.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5be70c6d-1fd6-464f-a7a0-345e4a4ca699
📒 Files selected for processing (2)
src/datamodel_code_generator/http.pytests/test_http.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_http.py
Assert httpx.get is not called when remote refs are blocked, preventing real HTTP leaks if the gate regresses. Assert the specific file path in the missing local ref error, not just the generic prefix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/main/jsonschema/test_main_jsonschema.py (1)
813-831: Consider extracting a small HTTP mock-response factory to reduce repetition.The repeated
Mock()+status_code+headers+textsetup appears many times; a helper would improve maintainability and reduce drift in future updates.Also applies to: 847-875, 889-904, 908-927, 939-960
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/main/jsonschema/test_main_jsonschema.py` around lines 813 - 831, Extract a small HTTP mock-response factory (e.g., make_http_response(text, status_code=200, headers=None)) and replace the repeated mocker.Mock() + status_code + headers + text setup in test_main_root_id_jsonschema_with_local_file and the other JSON Schema tests in the same file; update the tests to call the factory to create person_response/root_id_response and keep the existing httpx.get patching/assertions unchanged (reference the test function name test_main_root_id_jsonschema_with_local_file and the other JSON Schema tests in this module to locate all occurrences).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/main/jsonschema/test_main_jsonschema.py`:
- Around line 813-831: Extract a small HTTP mock-response factory (e.g.,
make_http_response(text, status_code=200, headers=None)) and replace the
repeated mocker.Mock() + status_code + headers + text setup in
test_main_root_id_jsonschema_with_local_file and the other JSON Schema tests in
the same file; update the tests to call the factory to create
person_response/root_id_response and keep the existing httpx.get
patching/assertions unchanged (reference the test function name
test_main_root_id_jsonschema_with_local_file and the other JSON Schema tests in
this module to locate all occurrences).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 04af1f13-5bd1-470c-8047-d56f66df7c5f
📒 Files selected for processing (1)
tests/main/jsonschema/test_main_jsonschema.py
Owner
|
@butvinm My concern is that changing the default to How about this instead? Introduce The error message improvements (clearer FileNotFoundError, HTML response validation) are great. I'd like to merge those regardless. |
Per maintainer feedback, keep backward compatibility by allowing remote $ref fetching by default but emit a FutureWarning when it happens without explicit --allow-remote-refs. The flag becomes a three-state: True (explicit opt-in, no warning), False (blocks), None (default, allows with deprecation warning). The default will flip to False in a future major version. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/datamodel_code_generator/__main__.py (1)
1085-1088: Consider respecting explicit user setting when--urlis provided.Currently,
allow_remote_refsis unconditionally set toTruewhenconfig.urlis provided. This would override an explicit--no-allow-remote-refssetting from the user. A user might want to fetch the main schema via--urlwhile still blocking remote$refresolution within that schema.♻️ Proposed fix to only set when not explicitly configured
# --url implies --allow-remote-refs since the user is explicitly fetching a remote schema - if config.url: + if config.url and config.allow_remote_refs is None: config.allow_remote_refs = True🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datamodel_code_generator/__main__.py` around lines 1085 - 1088, The current unconditional assignment in __main__.py sets config.allow_remote_refs = True whenever config.url is provided, which overrides an explicit user choice; change the logic so that when config.url is truthy you only set config.allow_remote_refs to True if the option was not explicitly provided by the user (e.g., check for a sentinel like None or a flag that indicates it was unset). Update the block around the config.url handling to test the explicitness of allow_remote_refs (e.g., if config.url and config.allow_remote_refs is None: config.allow_remote_refs = True) so an explicit --no-allow-remote-refs or --allow-remote-refs from the CLI is respected.tests/main/jsonschema/test_main_jsonschema.py (1)
985-1015: Assert the warning-path test actually exercised remote fetch.Line 994 patches the fetch function, but the test currently only checks for a
FutureWarning. Add a call assertion so this test proves implicit remote resolution happened.♻️ Suggested hardening
- mocker.patch("httpx.get", return_value=person_response) + httpx_get_mock = mocker.patch("httpx.get", return_value=person_response) @@ with pytest.warns(FutureWarning, match="--allow-remote-refs"): run_main_and_assert( input_path=input_file, output_path=output_file, input_file_type="jsonschema", ) + httpx_get_mock.assert_called()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/main/jsonschema/test_main_jsonschema.py` around lines 985 - 1015, The test test_main_remote_ref_emits_deprecation_warning patches httpx.get but never asserts it was invoked, so add an assertion after run_main_and_assert to verify the remote fetch actually happened; specifically, reference the patched "httpx.get" mock (or the mock object returned, e.g., person_response) and assert it was called (optionally with the expected URL "https://example.com/other/schema.json" or at least assert_called_once()) so the test proves implicit remote resolution occurred.src/datamodel_code_generator/parser/jsonschema.py (1)
3821-3837: Scope theallow_remote_refsgate to HTTP/HTTPS explicitly.Right now, Line 3821 only excludes
file://; all other URL schemes are treated as “remote HTTP” in gating/warning text. Using explicit scheme checks keeps behavior aligned with the flag semantics and messaging.Suggested patch
@@ -from urllib.parse import ParseResult, unquote +from urllib.parse import ParseResult, unquote, urlparse @@ def _get_ref_body(self, resolved_ref: str) -> dict[str, YamlValue]: """Get the body of a reference from URL or remote file.""" if is_url(resolved_ref): - if not resolved_ref.startswith("file://"): + scheme = urlparse(resolved_ref).scheme.lower() + is_http_ref = scheme in {"http", "https"} + if is_http_ref: if self.allow_remote_refs is False: msg = ( f"Fetching remote $ref is disabled: {resolved_ref}\n" "Use --allow-remote-refs to enable HTTP fetching of remote references." ) raise Error(msg) if self.allow_remote_refs is None: import warnings # noqa: PLC0415🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datamodel_code_generator/parser/jsonschema.py` around lines 3821 - 3837, The current gate uses only resolved_ref.startswith("file://") so non-file but non-HTTP schemes are incorrectly treated as remote HTTP; update the conditional around allow_remote_refs to explicitly check for HTTP/HTTPS (e.g., if resolved_ref.lower().startswith(("http://","https://")):) before applying the allow_remote_refs False error and the None warning. Modify the branch that references allow_remote_refs, resolved_ref and the warning/error text to run only for HTTP(S) refs so other schemes (data:, ftp:, etc.) are not affected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/datamodel_code_generator/__main__.py`:
- Around line 1085-1088: The current unconditional assignment in __main__.py
sets config.allow_remote_refs = True whenever config.url is provided, which
overrides an explicit user choice; change the logic so that when config.url is
truthy you only set config.allow_remote_refs to True if the option was not
explicitly provided by the user (e.g., check for a sentinel like None or a flag
that indicates it was unset). Update the block around the config.url handling to
test the explicitness of allow_remote_refs (e.g., if config.url and
config.allow_remote_refs is None: config.allow_remote_refs = True) so an
explicit --no-allow-remote-refs or --allow-remote-refs from the CLI is
respected.
In `@src/datamodel_code_generator/parser/jsonschema.py`:
- Around line 3821-3837: The current gate uses only
resolved_ref.startswith("file://") so non-file but non-HTTP schemes are
incorrectly treated as remote HTTP; update the conditional around
allow_remote_refs to explicitly check for HTTP/HTTPS (e.g., if
resolved_ref.lower().startswith(("http://","https://")):) before applying the
allow_remote_refs False error and the None warning. Modify the branch that
references allow_remote_refs, resolved_ref and the warning/error text to run
only for HTTP(S) refs so other schemes (data:, ftp:, etc.) are not affected.
In `@tests/main/jsonschema/test_main_jsonschema.py`:
- Around line 985-1015: The test test_main_remote_ref_emits_deprecation_warning
patches httpx.get but never asserts it was invoked, so add an assertion after
run_main_and_assert to verify the remote fetch actually happened; specifically,
reference the patched "httpx.get" mock (or the mock object returned, e.g.,
person_response) and assert it was called (optionally with the expected URL
"https://example.com/other/schema.json" or at least assert_called_once()) so the
test proves implicit remote resolution occurred.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c66d847b-730b-4788-8d60-4fefb8607ded
📒 Files selected for processing (10)
src/datamodel_code_generator/__main__.pysrc/datamodel_code_generator/_types/generate_config_dict.pysrc/datamodel_code_generator/_types/parser_config_dicts.pysrc/datamodel_code_generator/arguments.pysrc/datamodel_code_generator/config.pysrc/datamodel_code_generator/parser/base.pysrc/datamodel_code_generator/parser/jsonschema.pytests/data/expected/main/input_model/config_class.pytests/main/jsonschema/test_main_jsonschema.pytests/main/test_public_api_signature_baseline.py
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/main/test_public_api_signature_baseline.py
- src/datamodel_code_generator/parser/base.py
- src/datamodel_code_generator/_types/parser_config_dicts.py
- src/datamodel_code_generator/arguments.py
- src/datamodel_code_generator/_types/generate_config_dict.py
Contributor
Author
|
Thanks for the feedback! Updated the PR:
Example output for the original issue (schema with |
koxudaxi
previously approved these changes
Mar 19, 2026
Collaborator
|
@koxudaxi @butvinm what if we allow See: https://docs.python.org/3/library/argparse.html#argparse.BooleanOptionalAction |
Use BooleanOptionalAction (like --use-annotated) so users can explicitly opt out with --no-allow-remote-refs from the CLI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
BooleanOptionalAction registers both --allow-remote-refs and --no-allow-remote-refs in argparse; the sync test requires both to be in CLI_OPTION_META. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Breaking Change AnalysisResult: Breaking changes detected Reasoning: The PR introduces three categories of breaking changes: (1) Missing local $ref files now raise Error instead of FileNotFoundError - a change in exception type that affects programmatic callers. (2) HTTP failures now raise SchemaFetchError early instead of letting bad content propagate to the YAML/JSON parser, changing what exceptions surface for these error cases. (3) A FutureWarning is emitted for implicit remote ref fetching, which could break CI/CD pipelines with strict warning filters. The core functionality is preserved (remote refs still work by default), but the error handling surface has changed in ways that could affect programmatic users. Content for Release NotesError Handling Changes
Default Behavior Changes
This analysis was performed by Claude Code Action |
Contributor
|
🎉 Released in 0.56.0 This PR is now available in the latest release. See the release notes for details. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3039
When a JSON schema has a
$idwith an HTTP URL and a$refpointing to a non-existent local file, the tool silently resolved the ref against the$idURL, made an HTTP request, and produced a confusing YAML parse error on the HTML response.This PR adds
--allow-remote-refs/--no-allow-remote-refswith a deprecation path: remote fetching still works by default but emits aFutureWarningwhen it happens implicitly. In a future major version, the default will flip to disabled.Before (bug)
After (fix)
--allow-remote-refs/--no-allow-remote-refs(BooleanOptionalAction)FutureWarning— backward compatible--allow-remote-refs: explicit opt-in, no warning--no-allow-remote-refs: blocks remote refs with clear error--urlinput: implicitly enables--allow-remote-refs(no warning)Other improvements
"$ref file not found: <path>"instead of rawFileNotFoundErrorSchemaFetchError)SchemaFetchError🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests