Skip to content

refactor(BA-3590): Implement dto for req/res data validation for gradual migra… #7865

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
kwonkwonn wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

refactor(BA-3590): Implement dto for req/res data validation for gradual migra… #7865

kwonkwonn wants to merge 4 commits into from

Conversation

@kwonkwonn
Copy link
Contributor

@kwonkwonn kwonkwonn commented Jan 8, 2026 •
edited
Loading

resolves #7625 (BA-3590)

This PR should be merged after #7653 being merged and tested. please don't merge this for now.

should be rebased and tested before merging take place.

This PR would consists of sequence of commits following.

  • define DTO for req/res (can easily validate with manager/api/auth.py's res, req)
  • define Data class and expected actions behavior and form.

Real Migration will be taken from next pr.

Once this process

  • write test code(refering tests/unit/manager/api/test_resource)
  • Define DTO, DataClass, action validator.
  • Define Handler and Adapter.
    succesfully done, It can be easily tested and automatized.

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version
  • Mention to the original issue
  • Installer updates including:
    • Fixtures for db schema changes
    • New mandatory config options
  • Update of end-to-end CLI integration tests in ai.backend.test
  • API server-client counterparts (e.g., manager API -> client SDK)
  • Test case(s) to:
    • Demonstrate the difference of before/after
    • Demonstrate the flow of abstract/conceptual models with a concrete implementation
  • Documentation
    • Contents in the docs directory
    • docstrings in public interfaces and type annotations

@github-actions github-actions bot added size:L 100~500 LoC Intern-OKR comp:common Related to Common component labels Jan 8, 2026
@github-actions github-actions bot added the comp:manager Related to Manager component label Jan 8, 2026
@kwonkwonn kwonkwonn force-pushed the refactor/migrate-manager-auth-to-pydantic branch from 1f87e7d to 6f2f113 Compare January 8, 2026 07:49
@github-actions github-actions bot added size:XL 500~ LoC and removed size:L 100~500 LoC labels Jan 8, 2026
fregataa
fregataa requested changes Jan 8, 2026
View reviewed changes
src/ai/backend/manager/repositories/auth/repository.py
Comment on lines +103 to +118

@auth_repository_resilience.apply()
async def get_credential_by_access_key(self, access_key: str) -> Optional[CredentialData]:
"""
Get user credential data by access key.
Used by authentication middleware to populate request context.
Args:
access_key: The access key to look up.
Returns:
CredentialData containing user, keypair, and resource policies,
or None if not found or inactive.
"""
return await self._db_source.fetch_credential_by_access_key(access_key)
Copy link
Member

@fregataa fregataa Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this function used? I cannot find

Copy link
Contributor Author

@kwonkwonn kwonkwonn Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to use this function
I should change some structures in api/auth.py, but since test for auth.py is not being passed i postponed to modify auth.py for now.

Copy link
Contributor Author

@kwonkwonn kwonkwonn Jan 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or I can modify a little bit to use that function as it is only draft pr.
If you think it seems alright.

src/ai/backend/common/dto/manager/auth/__init__.py Outdated
Comment on lines 39 to 68
__all__ = (
# Types
"AuthTokenType",
"AuthResponseType",
"TwoFactorType",
"AuthResponse",
"AuthSuccessResponse",
"RequireTwoFactorRegistrationResponse",
"RequireTwoFactorAuthResponse",
"parse_auth_response",
# Request DTOs
"AuthorizeRequest",
"GetRoleRequest",
"SignupRequest",
"SignoutRequest",
"UpdateFullNameRequest",
"UpdatePasswordRequest",
"UpdatePasswordNoAuthRequest",
"UploadSSHKeypairRequest",
# Response DTOs
"AuthorizeResponse",
"GetRoleResponse",
"SignupResponse",
"SignoutResponse",
"UpdateFullNameResponse",
"UpdatePasswordResponse",
"UpdatePasswordNoAuthResponse",
"GetSSHKeypairResponse",
"SSHKeypairResponse",
)
Copy link
Member

@fregataa fregataa Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not export all modules in __init__.py.
Ref: #7702

Copy link
Contributor Author

@kwonkwonn kwonkwonn Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will modify to export only needed modules.
Thank you!

Copy link
Contributor Author

@kwonkwonn kwonkwonn Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed re-exporting from __init__py.
Please refer my second commit. ( as 'authadapter` imports each modules from init.py, i had to modify from second commit to ensure no conflicts to happen for each commit.)

@kwonkwonn kwonkwonn force-pushed the refactor/migrate-manager-auth-to-pydantic branch from 30118d3 to 44d48e5 Compare January 9, 2026 02:26
@github-actions github-actions bot added comp:client Related to Client component comp:webserver Related to Web Server component labels Jan 9, 2026
@kwonkwonn kwonkwonn force-pushed the refactor/migrate-manager-auth-to-pydantic branch from 44d48e5 to c159a1b Compare January 9, 2026 02:47
@github-actions github-actions bot added size:L 100~500 LoC and removed size:XL 500~ LoC labels Jan 9, 2026
@kwonkwonn kwonkwonn requested a review from fregataa January 9, 2026 02:51
@github-actions github-actions bot added size:XL 500~ LoC and removed size:L 100~500 LoC labels Jan 9, 2026
@kwonkwonn kwonkwonn force-pushed the refactor/migrate-manager-auth-to-pydantic branch from fabafe2 to dc34f55 Compare January 9, 2026 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@HyeockJinKim HyeockJinKim Awaiting requested review from HyeockJinKim

@fregataa fregataa Awaiting requested review from fregataa

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

Requested changes must be addressed to merge this pull request.

Assignees

@kwonkwonn kwonkwonn

Labels

comp:client Related to Client component comp:common Related to Common component comp:manager Related to Manager component comp:webserver Related to Web Server component Intern-OKR size:XL 500~ LoC

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Restructure auth.py into modular directory structure (auth/adapter, auth/handler, auth/models, auth/init.py)

3 participants

@kwonkwonn @fregataa