Add --openapi-include-paths option for path-based model filtering#2894
Add --openapi-include-paths option for path-based model filtering#2894
Conversation
|
Warning Rate limit exceeded@koxudaxi has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI Parser
participant Config as Config Layer
participant OpenAPIParser as OpenAPI Parser
participant PathFilter as Path Matching
User->>CLI: Run with --openapi-include-paths /pets* /store*
CLI->>Config: Store patterns in openapi_include_paths
Config->>OpenAPIParser: Pass openapi_include_paths to parser
alt Paths scope enabled
OpenAPIParser->>OpenAPIParser: Store patterns, no warning
else Paths scope not enabled
OpenAPIParser->>User: ⚠️ Warning: include-paths only work with paths scope
end
Note over OpenAPIParser: During path processing
loop For each OpenAPI path item
OpenAPIParser->>PathFilter: _normalize_path(path)
PathFilter-->>OpenAPIParser: normalized_path
alt apply_path_filter = True
OpenAPIParser->>PathFilter: _matches_path_pattern(normalized_path)
PathFilter-->>OpenAPIParser: matches (bool)
alt Pattern matches
OpenAPIParser->>OpenAPIParser: Process path item → Generate model
else Pattern does not match
OpenAPIParser->>OpenAPIParser: Skip path item
end
else apply_path_filter = False (e.g., webhooks)
OpenAPIParser->>OpenAPIParser: Process all items (no filtering)
end
end
OpenAPIParser-->>User: Generated code with filtered paths
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
|
📚 Docs Preview: https://pr-2894.datamodel-code-generator.pages.dev |
CodSpeed Performance ReportMerging #2894 will not alter performanceComparing
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
docs/cli-reference/openapi-only-options.md (1)
8-528: Docs look good; consider clarifying scope and pattern semanticsThe new table row and
--openapi-include-pathssection are consistent with how other OpenAPI options are documented and match the new tests/golden output.Two small clarity improvements would help users:
- Explicitly state that
--openapi-include-pathsonly has an effect when--openapi-scopesincludespaths(there is a runtime warning for this, and your example already usespaths schemas).- Mention that patterns are matched using fnmatch-style globbing (e.g.
/pets*,pets/*), to align the prose with the CLI help text.These can be added in the short description or just below it without changing any behavior.
tests/main/openapi/test_main_openapi.py (1)
4728-4749: Warning test is solid; innerwarningsimport is optionalThe warning assertion for using
--openapi-include-pathswithoutpathsin--openapi-scopesis well structured and matches the intended message text.Minor style nit: the function re-imports
warningslocally even though the module imports it at the top. You can safely drop the innerimport warningsand use the module-level import instead to keep things a bit cleaner.src/datamodel_code_generator/__main__.py (1)
554-554: Remove unusednoqadirective.The
# noqa: UP045directive is unnecessary here, as confirmed by Ruff static analysis.🔎 Proposed fix
- openapi_include_paths: Optional[list[str]] = None # noqa: UP045 + openapi_include_paths: Optional[list[str]] = None
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
docs/cli-reference/index.mddocs/cli-reference/openapi-only-options.mddocs/cli-reference/quick-reference.mdsrc/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/parser/openapi.pysrc/datamodel_code_generator/prompt_data.pytests/data/expected/main/input_model/config_class.pytests/data/expected/main/openapi/openapi_include_paths/pets_only.pytests/main/openapi/test_main_openapi.pytests/main/test_public_api_signature_baseline.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/datamodel_code_generator/parser/openapi.py (2)
src/datamodel_code_generator/enums.py (1)
OpenAPIScope(70-78)src/datamodel_code_generator/model/base.py (1)
path(908-910)
🪛 GitHub Actions: Test
src/datamodel_code_generator/__main__.py
[error] 868-868: OpenAPI path processing failed during code generation due to the above TypeError.
src/datamodel_code_generator/parser/openapi.py
[error] 359-359: TypeError: OpenAPIParser._normalize_path() takes 1 positional argument but 2 were given during path pattern matching (openapi.py:359). This occurred while running pytest for OpenAPI include-path tests (Command: pytest -n auto --color=yes -m 'not perf' ... tests).
[error] 741-741: Traceback indicates the TypeError originates in _normalize_path and bubbles up to parse_raw during OpenAPI parsing (openapi.py:741). Command: pytest -n auto --color=yes -m 'not perf' -p no:codspeed --benchmark-disable --cov ...
tests/main/openapi/test_main_openapi.py
[error] 4706-4706: OpenAPIParser._normalize_path() takes 1 positional argument but 2 were given. (Triggered during test_main_openapi_include_paths)
[error] 4718-4718: OpenAPIParser._normalize_path() takes 1 positional argument but 2 were given. (Triggered during test_main_openapi_include_paths_without_leading_slash)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/__main__.py
554-554: Unused noqa directive (non-enabled: UP045)
Remove unused noqa directive
(RUF100)
🔇 Additional comments (21)
tests/data/expected/main/openapi/openapi_include_paths/pets_only.py (1)
1-27: LGTM! Well-structured test fixture.The expected output file demonstrates clean model generation with proper type annotations and modern Python syntax. The models correctly represent filtered OpenAPI path schemas.
src/datamodel_code_generator/config.py (2)
143-143: LGTM! Clean config field addition.The
openapi_include_pathsfield is properly typed and positioned logically alongside related OpenAPI options. The optional list type with None default is appropriate for this filtering feature.
352-352: LGTM! Parser config properly extended.The field addition to
OpenAPIParserConfigcorrectly mirrors theGenerateConfigfield, ensuring the option is available at the parser level for filtering logic.docs/cli-reference/quick-reference.md (2)
158-158: LGTM! Documentation properly updated.The new option is correctly added to the OpenAPI-only Options table with appropriate description and linking.
278-278: LGTM! Alphabetical index updated correctly.The option is properly inserted in alphabetical order within the index with consistent formatting and linking.
src/datamodel_code_generator/__init__.py (1)
742-742: LGTM! Option correctly propagated to parser.The
openapi_include_pathsconfiguration is properly added to the OpenAPI parser options dictionary, ensuring the filtering configuration reaches the parsing layer where it's needed.docs/cli-reference/index.md (2)
16-16: LGTM! Option count updated correctly.The OpenAPI-only Options count is accurately incremented from 6 to 7 to reflect the new
--openapi-include-pathsoption.
139-139: LGTM! Index entry properly added.The new option is correctly placed in alphabetical order within the O section with proper formatting and linking to the detailed documentation.
src/datamodel_code_generator/_types/generate_config_dict.py (1)
105-105: LGTM!The new field addition is well-typed and consistently placed with other OpenAPI configuration options.
src/datamodel_code_generator/_types/parser_config_dicts.py (1)
165-165: LGTM!The field addition to
OpenAPIParserConfigDictis consistent with the corresponding change inGenerateConfigDictand properly placed within the OpenAPI-specific configuration.tests/data/expected/main/input_model/config_class.py (1)
178-178: LGTM!The test expectation correctly reflects the new configuration field with consistent type annotation and placement.
src/datamodel_code_generator/cli_options.py (1)
239-239: LGTM!The CLI option metadata entry is correctly categorized and positioned with other OpenAPI-specific options.
src/datamodel_code_generator/arguments.py (1)
927-938: Scope enforcement verified.The CLI argument definition is well-structured with clear help text, appropriate parameter configuration (
nargs="+"for multiple patterns), and explicit documentation of the scope requirement. The fnmatch pattern examples are helpful for users.Scope enforcement is confirmed: when
--openapi-include-pathsis used without'paths'in--openapi-scopes, the code issues a warning at initialization time (src/datamodel_code_generator/parser/openapi.py:213) with the message "--openapi-include-paths has no effect without --openapi-scopes paths". Test coverage exists for this behavior.src/datamodel_code_generator/prompt_data.py (1)
82-82: New option description is consistent and well‑placed
"--openapi-include-paths": "Filter OpenAPI paths to include in model generation."matches the CLI help and nearby OpenAPI options; no issues from a prompt-data/metadata perspective.tests/main/test_public_api_signature_baseline.py (1)
56-125: Baseline signature update foropenapi_include_pathsis alignedAdding
openapi_include_paths: list[str] | None = Noneto_baseline_generatekeeps the baseline in sync with the new GenerateConfig field and matches the intended type/default. The surrounding tests (test_generate_signature_matches_baseline, default checks) will enforce consistency withGenerateConfigDictandGenerateConfig, so this change is correctly wired into the signature contract.tests/main/openapi/test_main_openapi.py (1)
4716-4725: Good coverage for patterns without leading slash
test_main_openapi_include_paths_without_leading_slashnicely verifies that patterns like"pets*"are normalized the same as"/pets*"and still filter to thepets_only.pygolden output. This is valuable coverage for user ergonomics around leading slashes in path patterns.src/datamodel_code_generator/__main__.py (1)
932-932: LGTM!The new
openapi_include_pathsparameter is correctly forwarded to the code generation function, following the established pattern for other configuration options.src/datamodel_code_generator/parser/openapi.py (4)
9-9: LGTM!The
fnmatchmodule is the appropriate choice for implementing shell-style wildcard pattern matching for OpenAPI paths.
212-217: LGTM!Good validation logic to warn users when
--openapi-include-pathsis specified but would have no effect because thepathsscope is not enabled.
373-379: LGTM!The conditional path filtering implementation is well-designed. The
apply_path_filterparameter allows selective filtering, which is essential for handling webhooks (which should not be filtered) differently from paths (which should be filtered).
745-747: LGTM!Correctly excludes webhooks from path filtering by passing
apply_path_filter=False. This is the appropriate behavior since webhooks are conceptually distinct from API paths and the filtering option is specifically named--openapi-include-paths.
koxudaxi
force-pushed
the
feature/openapi-include-paths
branch
from
240c575 to
2f6ac48
Compare
koxudaxi
force-pushed
the
feature/openapi-include-paths
branch
from
bae8068 to
4a7eb0a
Compare
🤖 Generated by GitHub Actions
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2894 +/- ##
=======================================
Coverage 99.37% 99.37%
=======================================
Files 92 92
Lines 16196 16228 +32
Branches 1915 1919 +4
=======================================
+ Hits 16095 16127 +32
Misses 52 52
Partials 49 49
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:
|
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR adds a new optional CLI flag This analysis was performed by Claude Code Action |
|
🎉 Released in 0.52.0 This PR is now available in the latest release. See the release notes for details. |
2 participants
Fixes: #2284
Summary by CodeRabbit
New Features
--openapi-include-pathsCLI option to selectively include OpenAPI paths in model generation using fnmatch-style pattern matching. Includes validation to ensure the Paths scope is enabled.Documentation
--openapi-include-pathsoption.✏️ Tip: You can customize this high-level summary in your review settings.