refactor(BA-4911): migrate ScalingGroupOpts from attrs/trafaret to Pydantic BaseModel#9723
Open
HyeockJinKim wants to merge 5 commits intomainfrom
Open
refactor(BA-4911): migrate ScalingGroupOpts from attrs/trafaret to Pydantic BaseModel#9723HyeockJinKim wants to merge 5 commits intomainfrom
HyeockJinKim wants to merge 5 commits intomainfrom
Conversation
HyeockJinKim
added a commit
that referenced
this pull request
Mar 5, 2026
github-actions
bot
added
size:L
HyeockJinKim
commented
Mar 6, 2026
Comment on lines
+110
to
+114
| @classmethod | ||
| def from_json(cls, obj: Mapping[str, Any]) -> ScalingGroupOpts: | ||
| return cls(**cls.as_trafaret().check(obj)) | ||
| def validate_allowed_session_types(cls, value: Any) -> list[SessionTypes]: | ||
| if not isinstance(value, list): | ||
| raise ValueError(f"Expected a list, got {type(value)}") | ||
| return [SessionTypes(v) if isinstance(v, str) else v for v in value] |
Collaborator
Author
There was a problem hiding this comment.
Review whether this is the required validator.
Comment on lines
+120
to
+127
| @field_validator("pending_timeout", mode="before") | ||
| @classmethod | ||
| def as_trafaret(cls) -> t.Trafaret: | ||
| return t.Dict({ | ||
| t.Key("allowed_session_types", default=["interactive", "batch"]): t.List( | ||
| tx.Enum(SessionTypes), min_length=1 | ||
| ), | ||
| t.Key("pending_timeout", default=0): tx.TimeDuration(allow_negative=False), | ||
| # Each scheduler impl refers an additional "config" key. | ||
| t.Key("config", default={}): t.Mapping(t.String, t.Any), | ||
| t.Key("agent_selection_strategy", default=AgentSelectionStrategy.DISPERSED): tx.Enum( | ||
| AgentSelectionStrategy | ||
| ), | ||
| t.Key("agent_selector_config", default={}): agent_selector_config_iv, | ||
| t.Key("enforce_spreading_endpoint_replica", default=False): t.ToBool, | ||
| t.Key("allow_fractional_resource_fragmentation", default=True): t.ToBool, | ||
| t.Key("route_cleanup_target_statuses", default=["unhealthy"]): t.List( | ||
| t.Enum("healthy", "unhealthy", "degraded") | ||
| ), | ||
| }).allow_extra("*") | ||
| def validate_pending_timeout(cls, value: Any) -> timedelta: | ||
| if isinstance(value, (int, float)): | ||
| return timedelta(seconds=value) | ||
| if isinstance(value, timedelta): | ||
| return value | ||
| raise ValueError(f"Expected a number or timedelta, got {type(value)}") |
Collaborator
Author
There was a problem hiding this comment.
Review whether this is also essential.
…dantic BaseModel
- Replace @attr.define with Pydantic BaseModel (frozen=True)
- Add field validators for pending_timeout (float→timedelta) and
allowed_session_types (str list→SessionTypes list)
- Add field serializers for pending_timeout (→float), allowed_session_types
(→str list), and agent_selection_strategy (→str)
- Remove to_json(), from_json(), as_trafaret() methods
- Change scheduler_opts column type from StructuredJSONObjectColumn to PydanticColumn
- Update default from {} to ScalingGroupOpts (callable factory)
- Replace to_json()/from_json() calls in gql_legacy with model_dump()/model_validate()
- Remove unused imports (attr, trafaret, JSONSerializableMixin, StructuredJSONObjectColumn)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Test roundtrip serialization, default instantiation, field validators (pending_timeout timedelta, allowed_session_types enum, agent_selection_strategy), invalid input validation errors, and backward compatibility with legacy JSON data. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pydantic v2 natively handles str→enum coercion for string-backed enums and int/float→timedelta coercion, making the custom `mode="before"` validators unnecessary. Serializers are retained for DB-compatible JSON output format (float seconds, string enum values). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Summary
@attr.define+JSONSerializableMixin+ trafaret with PydanticBaseModel(frozen=True)forScalingGroupOptsStructuredJSONObjectColumn(ScalingGroupOpts)toPydanticColumn(ScalingGroupOpts)inScalingGroupRowmodel_validate()/model_dump(mode="json")instead offrom_json()/to_json()pending_timeout(timedelta↔float),allowed_session_types(enum↔str), andagent_selection_strategy(enum↔str)Test plan
tests/unit/manager/models/scaling_group/test_row.py(19 tests)pants fmt/fix/lintall passResolves BA-4911