Skip to content
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.

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

Fix pydantic v2 warnings related to UserGroup #572

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions api/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
from pymongo.errors import DuplicateKeyError
from fastapi_users import FastAPIUsers
from beanie import PydanticObjectId
from kernelci.api.models import (

Check failure on line 36 in api/main.py

View workflow job for this annotation

GitHub Actions / Lint

Unable to import 'kernelci.api.models'
Node,
Hierarchy,
PublishEvent,
Expand All @@ -52,7 +52,9 @@
User,
UserRead,
UserCreate,
UserCreateRequest,
UserUpdate,
UserUpdateRequest,
UserGroup,
)
from .metrics import Metrics
Expand Down Expand Up @@ -157,7 +159,7 @@

@app.post("/user/register", response_model=UserRead, tags=["user"],
response_model_by_alias=False)
async def register(request: Request, user: UserCreate,
async def register(request: Request, user: UserCreateRequest,
current_user: User = Depends(get_current_superuser)):
"""User registration route

Expand All @@ -183,9 +185,11 @@
detail=f"User group does not exist with name: \
{group_name}")
groups.append(group)
user.groups = groups
user_create = UserCreate(**(user.model_dump(
exclude={'groups'}, exclude_none=True)))
user_create.groups = groups
created_user = await register_router.routes[0].endpoint(
request, user, user_manager)
request, user_create, user_manager)
# Update user to be an admin user explicitly if requested as
# `fastapi-users` register route does not allow it
if user.is_superuser:
Expand Down Expand Up @@ -233,7 +237,7 @@

@app.patch("/user/me", response_model=UserRead, tags=["user"],
response_model_by_alias=False)
async def update_me(request: Request, user: UserUpdate,
async def update_me(request: Request, user: UserUpdateRequest,
current_user: User = Depends(get_current_user)):
"""User update route

Expand All @@ -258,15 +262,17 @@
detail=f"User group does not exist with name: \
{group_name}")
groups.append(group)
user_update = UserUpdate(**(user.model_dump(
exclude={'groups'}, exclude_none=True)))
if groups:
user.groups = groups
user_update.groups = groups
return await users_router.routes[1].endpoint(
request, user, current_user, user_manager)
request, user_update, current_user, user_manager)


@app.patch("/user/{user_id}", response_model=UserRead, tags=["user"],
response_model_by_alias=False)
async def update_user(user_id: str, request: Request, user: UserUpdate,
async def update_user(user_id: str, request: Request, user: UserUpdateRequest,
current_user: User = Depends(get_current_superuser)):
"""Router to allow admin users to update other user account"""
metrics.add('http_requests_total', 1)
Expand Down Expand Up @@ -295,11 +301,14 @@
detail=f"User group does not exist with name: \
{group_name}")
groups.append(group)
user_update = UserUpdate(**(user.model_dump(
exclude={'groups'}, exclude_none=True)))

if groups:
user.groups = groups
user_update.groups = groups

updated_user = await users_router.routes[3].endpoint(
user, request, user_from_id, user_manager
user_update, request, user_from_id, user_manager
)
# Update user to be an admin user explicitly if requested as
# `fastapi-users` user update route does not allow it
Expand Down
37 changes: 33 additions & 4 deletions api/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
Document,
PydanticObjectId,
)
from kernelci.api.models_base import DatabaseModel, ModelId

Check failure on line 31 in api/models.py

View workflow job for this annotation

GitHub Actions / Lint

Unable to import 'kernelci.api.models_base'


# PubSub model definitions
Expand Down Expand Up @@ -87,7 +87,7 @@
)

@field_validator('groups')
def validate_groups(cls, groups): # pylint: disable=no-self-argument

Check warning on line 90 in api/models.py

View workflow job for this annotation

GitHub Actions / Lint

Method could be a function
"""Unique group constraint"""
unique_names = {group.name for group in groups}
if len(unique_names) != len(groups):
Expand All @@ -107,13 +107,13 @@
]


class UserRead(schemas.BaseUser[PydanticObjectId], ModelId):

Check failure on line 110 in api/models.py

View workflow job for this annotation

GitHub Actions / Lint

Inheriting 'schemas.BaseUser[PydanticObjectId]', which is not a class.
"""Schema for reading a user"""
username: Annotated[str, Indexed(unique=True)]
groups: List[UserGroup] = Field(default=[])

@field_validator('groups')
def validate_groups(cls, groups): # pylint: disable=no-self-argument

Check warning on line 116 in api/models.py

View workflow job for this annotation

GitHub Actions / Lint

Method could be a function
"""Unique group constraint"""
unique_names = {group.name for group in groups}
if len(unique_names) != len(groups):
Expand All @@ -121,13 +121,13 @@
return groups


class UserCreate(schemas.BaseUserCreate):
"""Schema for creating a user"""
class UserCreateRequest(schemas.BaseUserCreate):
"""Create user request schema for API router"""
username: Annotated[str, Indexed(unique=True)]
groups: List[str] = Field(default=[])

@field_validator('groups')
def validate_groups(cls, groups): # pylint: disable=no-self-argument

Check warning on line 130 in api/models.py

View workflow job for this annotation

GitHub Actions / Lint

Method could be a function
"""Unique group constraint"""
unique_names = set(groups)
if len(unique_names) != len(groups):
Expand All @@ -135,14 +135,28 @@
return groups


class UserUpdate(schemas.BaseUserUpdate):
"""Schema for updating a user"""
class UserCreate(schemas.BaseUserCreate):
"""Schema used for sending create user request to 'fastapi-users' router"""
username: Annotated[str, Indexed(unique=True)]
groups: List[UserGroup] = Field(default=[])

@field_validator('groups')
def validate_groups(cls, groups): # pylint: disable=no-self-argument

Check warning on line 144 in api/models.py

View workflow job for this annotation

GitHub Actions / Lint

Method could be a function
"""Unique group constraint"""
unique_names = {group.name for group in groups}
if len(unique_names) != len(groups):
raise ValueError("Groups must have unique names.")
return groups


class UserUpdateRequest(schemas.BaseUserUpdate):
"""Update user request schema for API router"""
username: Annotated[Optional[str], Indexed(unique=True),
Field(default=None)]
groups: List[str] = Field(default=[])

@field_validator('groups')
def validate_groups(cls, groups): # pylint: disable=no-self-argument

Check warning on line 159 in api/models.py

View workflow job for this annotation

GitHub Actions / Lint

Method could be a function
"""Unique group constraint"""
unique_names = set(groups)
if len(unique_names) != len(groups):
Expand All @@ -150,6 +164,21 @@
return groups


class UserUpdate(schemas.BaseUserUpdate):
"""Schema used for sending update user request to 'fastapi-users' router"""
username: Annotated[Optional[str], Indexed(unique=True),
Field(default=None)]
groups: List[UserGroup] = Field(default=[])

@field_validator('groups')
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be convenient to reuse field validators but such discussion has never been resolved

Maybe the possible approaches to this task could be explored in future

Copy link
Collaborator Author

@JenySadadia JenySadadia Nov 27, 2024

Choose a reason for hiding this comment

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

Thanks for the input. Created #579

def validate_groups(cls, groups): # pylint: disable=no-self-argument

Check warning on line 174 in api/models.py

View workflow job for this annotation

GitHub Actions / Lint

Method could be a function
"""Unique group constraint"""
unique_names = {group.name for group in groups}
if len(unique_names) != len(groups):
raise ValueError("Groups must have unique names.")
return groups


# Pagination models

class CustomLimitOffsetParams(LimitOffsetParams):
Expand Down
Loading