refactor(api): migrate workspace account marshal_with responses to BaseModel#35190
Conversation
Pyrefly DiffNo changes detected. |
There was a problem hiding this comment.
Pull request overview
Refactors the console workspace account controller to replace remaining Flask-RESTX @marshal_with response handling with Pydantic ResponseModel DTOs, while keeping timestamp fields serialized as unix timestamps.
Changes:
- Replaced RESTX dict/field models for account integrates + education endpoints with Pydantic
ResponseModelresponses. - Added timestamp normalization via Pydantic validators (
created_at,expire_at). - Updated endpoint decorators to use
console_ns.response(..., console_ns.models[...])and returnmodel_dump(mode="json").
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # convert expire_at to UTC timestamp from isoformat | ||
| if res and "expire_at" in res: | ||
| res["expire_at"] = datetime.fromisoformat(res["expire_at"]).astimezone(pytz.utc) |
There was a problem hiding this comment.
EducationIdentity.status() is wrapped with or {} and then expire_at is unconditionally parsed via datetime.fromisoformat(...). If the billing API returns expire_at: null (or any non-string type), fromisoformat will raise a TypeError. Consider only parsing when expire_at is a non-empty str (and handle common ISO forms like a trailing Z), otherwise leave it as-is for the expire_at field validator to normalize when it’s already a datetime/timestamp.
| # convert expire_at to UTC timestamp from isoformat | |
| if res and "expire_at" in res: | |
| res["expire_at"] = datetime.fromisoformat(res["expire_at"]).astimezone(pytz.utc) | |
| # convert expire_at to UTC timestamp from isoformat when it is a non-empty string | |
| if res and "expire_at" in res: | |
| expire_at = res["expire_at"] | |
| if isinstance(expire_at, str) and expire_at: | |
| if expire_at.endswith("Z"): | |
| expire_at = f"{expire_at[:-1]}+00:00" | |
| res["expire_at"] = datetime.fromisoformat(expire_at).astimezone(pytz.utc) |
| return EducationAutocompleteResponse.model_validate( | ||
| BillingService.EducationIdentity.autocomplete(args.keywords, args.page, args.limit) or {} | ||
| ).model_dump(mode="json") | ||
|
|
||
|
|
There was a problem hiding this comment.
EducationAutocompleteResponse defaults data to an empty list and ignores unknown keys (ResponseModel.extra = "ignore"). If the billing API returns a different top-level key (e.g. institutions as used in api/tests/unit_tests/services/test_billing_service.py), this endpoint will now silently return {data: []} instead of surfacing the mismatch. Consider explicitly transforming the billing response into the {data, curr_page, has_next} shape (or adding field aliases / a validator to map known alternative keys) so a schema mismatch doesn’t become a silent empty result.
| return EducationAutocompleteResponse.model_validate( | |
| BillingService.EducationIdentity.autocomplete(args.keywords, args.page, args.limit) or {} | |
| ).model_dump(mode="json") | |
| raw_res = BillingService.EducationIdentity.autocomplete(args.keywords, args.page, args.limit) or {} | |
| if not isinstance(raw_res, dict): | |
| raise ValueError("Invalid education autocomplete response") | |
| if "data" in raw_res: | |
| res = { | |
| "data": raw_res["data"], | |
| "curr_page": raw_res.get("curr_page"), | |
| "has_next": raw_res.get("has_next"), | |
| } | |
| elif "institutions" in raw_res: | |
| res = { | |
| "data": raw_res["institutions"], | |
| "curr_page": raw_res.get("curr_page"), | |
| "has_next": raw_res.get("has_next"), | |
| } | |
| elif raw_res: | |
| raise ValueError("Unexpected education autocomplete response schema") | |
| else: | |
| res = {} | |
| return EducationAutocompleteResponse.model_validate(res).model_dump(mode="json") |
| account, _ = current_account_with_tenant() | ||
|
|
||
| return BillingService.EducationIdentity.verify(account.id, account.email) | ||
| return EducationVerifyResponse.model_validate( | ||
| BillingService.EducationIdentity.verify(account.id, account.email) or {} | ||
| ).model_dump(mode="json") |
There was a problem hiding this comment.
Education verify/status/autocomplete response paths were refactored to use Pydantic response models and timestamp normalization, but the controller unit tests in api/tests/unit_tests/controllers/console/workspace/test_accounts.py don’t cover these endpoints (it currently stops at AccountIntegrateApi). Adding tests for these endpoints would help catch schema/key mismatches and ensure expire_at stays normalized as expected.
…seModel (langgenius#35190) Co-authored-by: ai-hpc <ai-hpc@users.noreply.github.com>
…seModel (langgenius#35190) Co-authored-by: ai-hpc <ai-hpc@users.noreply.github.com>
Summary
@marshal_withresponse paths inapi/controllers/console/workspace/account.pyto Pydantic response modelsResponseModelDTOsRelated
Validation
ruff check api/controllers/console/workspace/account.pypython3 -m py_compile api/controllers/console/workspace/account.pypytest -q api/tests/unit_tests/controllers/console/workspace/test_accounts.py(fails in this environment: missinggraphondependency)