Open
Conversation
Replace pydantic v1 APIs with their v2 equivalents: - BaseSettings now comes from pydantic-settings - @validator becomes @field_validator - .dict()/.json() become .model_dump()/.model_dump_json() - __fields__ becomes model_fields - Result.__init__ becomes model_post_init - Mutable defaults wrapped in Field(default_factory=...) - _Settings proxy no longer inherits BaseSettings Fixes robusta-dev#244
|
|
WalkthroughThis pull request migrates the codebase from Pydantic v1 to v2 by updating dependency constraints, replacing deprecated APIs with v2 equivalents (validators, serialization methods, schema introspection), moving Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
13 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
robusta_krr/core/models/config.py (1)
124-136:⚠️ Potential issue | 🟡 MinorMissing return for the
"*"case invalidate_namespaces.When
vequals"*", the function falls through to line 135 which calls.lower()on a list, but the"*"string literal should be returned early. The condition on line 127 only handles empty list, not"*".🐛 Proposed fix
`@field_validator`("namespaces") `@classmethod` def validate_namespaces(cls, v: Union[list[str], Literal["*"]]) -> Union[list[str], Literal["*"]]: if v == []: return "*" + if v == "*": + return v if isinstance(v, list): for val in v: if val.startswith("*"): raise ValueError("Namespace's values cannot start with an asterisk (*)") return [val.lower() for val in v]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@robusta_krr/core/models/config.py` around lines 124 - 136, In validate_namespaces (the `@field_validator` on "namespaces") add an explicit early return when v == "*" so the literal asterisk is returned unchanged; keep the existing empty-list-to-"*" behavior, and for the list branch (isinstance(v, list)) validate that no element startswith("*") and then return [val.lower() for val in v]; this ensures the function never falls through and attempts .lower() on a non-list value and preserves the Union[list[str], Literal["*"]] return type.robusta_krr/core/runner.py (1)
524-524:⚠️ Potential issue | 🟠 MajorMissed Pydantic v2 migration:
.json()should be.model_dump_json().This line still uses the deprecated Pydantic v1
.json()method while the rest of the PR migrates to v2 APIs. This will cause a deprecation warning or failure.🐛 Proposed fix
- result_dict = json.loads(result.json(indent=2)) + result_dict = json.loads(result.model_dump_json(indent=2))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@robusta_krr/core/runner.py` at line 524, Replace the deprecated Pydantic v1 call result.json() with the v2 API result.model_dump_json() where result is the Pydantic model being serialized (the expression currently assigned to result_dict); keep the surrounding json.loads(...) call if you need a dict, or better yet call result.model_dump() to get a dict directly—update the expression that creates result_dict accordingly (e.g., use json.loads(result.model_dump_json(...)) or result.model_dump()) so it uses Pydantic v2.
🧹 Nitpick comments (1)
robusta_krr/formatters/yaml.py (1)
11-11: Consider usingmodel_dump()directly to avoid unnecessary JSON round-trip.The current approach serializes to JSON string then parses it back to a dict. Since
yaml_module.dump()accepts a dict, you can usemodel_dump()directly for better efficiency.♻️ Suggested optimization
- return yaml_module.dump(json.loads(result.model_dump_json()), sort_keys=False) + return yaml_module.dump(result.model_dump(), sort_keys=False)This also allows removing the unused
jsonimport at line 1.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@robusta_krr/formatters/yaml.py` at line 11, Replace the JSON round-trip in the YAML formatter by passing the model dict directly to yaml_module.dump: use result.model_dump() instead of json.loads(result.model_dump_json()) inside the return statement (update the call site that currently references model_dump_json), and remove the now-unused json import; ensure you keep sort_keys=False on yaml_module.dump.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@robusta_krr/core/models/config.py`:
- Around line 124-136: In validate_namespaces (the `@field_validator` on
"namespaces") add an explicit early return when v == "*" so the literal asterisk
is returned unchanged; keep the existing empty-list-to-"*" behavior, and for the
list branch (isinstance(v, list)) validate that no element startswith("*") and
then return [val.lower() for val in v]; this ensures the function never falls
through and attempts .lower() on a non-list value and preserves the
Union[list[str], Literal["*"]] return type.
In `@robusta_krr/core/runner.py`:
- Line 524: Replace the deprecated Pydantic v1 call result.json() with the v2
API result.model_dump_json() where result is the Pydantic model being serialized
(the expression currently assigned to result_dict); keep the surrounding
json.loads(...) call if you need a dict, or better yet call result.model_dump()
to get a dict directly—update the expression that creates result_dict
accordingly (e.g., use json.loads(result.model_dump_json(...)) or
result.model_dump()) so it uses Pydantic v2.
---
Nitpick comments:
In `@robusta_krr/formatters/yaml.py`:
- Line 11: Replace the JSON round-trip in the YAML formatter by passing the
model dict directly to yaml_module.dump: use result.model_dump() instead of
json.loads(result.model_dump_json()) inside the return statement (update the
call site that currently references model_dump_json), and remove the now-unused
json import; ensure you keep sort_keys=False on yaml_module.dump.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5436bea6-317b-4b21-9c53-1d8091ba3b29
📒 Files selected for processing (10)
pyproject.tomlrequirements.txtrobusta_krr/core/models/allocations.pyrobusta_krr/core/models/config.pyrobusta_krr/core/models/objects.pyrobusta_krr/core/models/result.pyrobusta_krr/core/runner.pyrobusta_krr/formatters/pprint.pyrobusta_krr/formatters/yaml.pyrobusta_krr/main.py
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
2 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.
Distributions that only ship Pydantic v2 can't run KRR because it pins pydantic v1. This brings the main app in line with the enforcer module, which was already on v2.
What changed:
pydanticbumped to>=2.0,<3.0, addedpydantic-settingsforBaseSettings@validator->@field_validatorwith@classmethod(7 in config.py, 1 in allocations.py).dict()/.json()->.model_dump()/.model_dump_json()(3 call sites)__fields__->model_fieldsin the CLI generation code in main.pyResult.__init__->model_post_initset(),{}) wrapped inField(default_factory=...)_Settingsproxy no longer inheritsBaseSettings— it just proxies via__getattr__, so the inheritance was unnecessary and breaks under v2's stricter initFixes #244