Improve aliases for Pydantic models#22708
Open
blubber wants to merge 1 commit intoOpenAPITools:masterfrom
Open
Improve aliases for Pydantic models#22708blubber wants to merge 1 commit intoOpenAPITools:masterfrom
blubber wants to merge 1 commit intoOpenAPITools:masterfrom
Conversation
The pydantic models generated include field aliases for fields that use
camel case as opposed to snake case (customary in Python). The alias
definitions are compatible with python's builtin dataclasses and are
correctly detected by LSPs, however in the same models the
`populate_by_name` option is set to True. This option allows the
instantiotion using the actual field names (snake case) instead of the
defined alias (camcel case). This options is not correctly recognized by
most LSP and therefore results in two warnings per field. The first
warning states that hte snake case version of the field is not a valid
argument, and the second warning states that the camcel case version of
the argument is missing.
Exampel:
```python
class WebSessionInfo(BaseModel):
model_config = {
"populate_by_name": True,
}
session_id: str = Field(alias="sessionID")
```
The above model can be instantiated like so:
`WebSessionInfo(session_id='abc')`, however this results in two
warnings (`session_id` is not a valid argument, and `sessionID` is
missing).
Settings `validation_alias` and `serialization_alias` results in the
exact same behavior as far as validation and serialization are
concerned, however they are not standard dataclass options and are
therefore not recognized by LSPs. This removes the warnings.
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonPydanticV1Codegen.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonPydanticV1Codegen.java:911">
P1: Pydantic v1 generator emits v2-only Field kwargs (`validation_alias`/`serialization_alias`), causing runtime TypeError and alias breakage.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+911
to
+912
| fields.add(String.format(Locale.ROOT, "validation_alias=\"%s\"", cp.baseName)); | ||
| fields.add(String.format(Locale.ROOT, "serialization_alias=\"%s\"", cp.baseName)); |
There was a problem hiding this comment.
P1: Pydantic v1 generator emits v2-only Field kwargs (validation_alias/serialization_alias), causing runtime TypeError and alias breakage.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonPydanticV1Codegen.java, line 911:
<comment>Pydantic v1 generator emits v2-only Field kwargs (`validation_alias`/`serialization_alias`), causing runtime TypeError and alias breakage.</comment>
<file context>
@@ -908,7 +908,8 @@ private ModelsMap postProcessModelsMap(ModelsMap objs) {
// field
if (cp.baseName != null && !cp.baseName.equals(cp.name)) { // base name not the same as name
- fields.add(String.format(Locale.ROOT, "alias=\"%s\"", cp.baseName));
+ fields.add(String.format(Locale.ROOT, "validation_alias=\"%s\"", cp.baseName));
+ fields.add(String.format(Locale.ROOT, "serialization_alias=\"%s\"", cp.baseName));
}
</file context>
Suggested change
| fields.add(String.format(Locale.ROOT, "validation_alias=\"%s\"", cp.baseName)); | |
| fields.add(String.format(Locale.ROOT, "serialization_alias=\"%s\"", cp.baseName)); | |
| fields.add(String.format(Locale.ROOT, "alias=\"%s\"", cp.baseName)); |
Member
|
thanks for the pr please follow step 3 to update the samples |
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
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.
The pydantic models generated include field aliases for fields that use camel case as opposed to snake case (customary in Python). The alias definitions are compatible with python's builtin dataclasses and are correctly detected by LSPs, however in the same models the
populate_by_nameoption is set to True. This option allows the instantiotion using the actual field names (snake case) instead of the defined alias (camcel case). This options is not correctly recognized by most LSP and therefore results in two warnings per field. The first warning states that hte snake case version of the field is not a valid argument, and the second warning states that the camcel case version of the argument is missing.Exampel:
The above model can be instantiated like so:
WebSessionInfo(session_id='abc'), however this results in two warnings (session_idis not a valid argument, andsessionIDis missing).Settings
validation_aliasandserialization_aliasresults in the exact same behavior as far as validation and serialization are concerned, however they are not standard dataclass options and are therefore not recognized by LSPs. This removes the warnings.PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)Summary by cubic
Switch generated Pydantic v1 fields to use validation_alias and serialization_alias instead of alias to prevent LSP warnings when populate_by_name=True. Validation and serialization behavior is unchanged; snake_case init and camelCase I/O still work.
Written for commit ba30bdc. Summary will update on new commits.