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

Add the skeleton interface of VFolder CRUD APIs using the new layered architecture in Manager #3433

Closed
HyeockJinKim opened this issue Jan 13, 2025 — with Lablup-Issue-Syncer · 0 comments · Fixed by #3493
Assignees

Comments

@HyeockJinKim
Copy link
Collaborator

HyeockJinKim commented Jan 13, 2025

Motivation

  • The vFolder CRUD handler in manager contains significant code duplication and lacks clarity, making it difficult to maintain.
  • The interface requires refactoring to enhance modularity and scalability, which will facilitate smoother integration of future features and updates.

Required Features

  • Define Handler container class and method
  • Split all existing super-role handlers in /manger/api/vfolder.py into Service and Repository layers for each interest
  • Apply Handler’s method to new route

Implementation Detail (interface)

Using @dataclass , Pydantic for data handling, validation

Transitioning from the Trafaret library and raw Python data types to @dataclass and Pydantic for data handling and validation. This update strengthens type hints, improves validation, and preserves existing functionality. The goal is to enhance code readability, type safety, and IDE support, aligning with modern Python best practices.

@dataclass
class UserIdentity:
    user_uuid: uuid.UUID
    user_role: str
    domain_name: str

@dataclass
class UserGroupIdentity:
    group_id: uuid.UUID | None
    group_name: str | None

@dataclass
class Keypair:
    access_key: str
    resource_policy: Mapping[str, Any]
    
class BaseSchema(BaseModel):
    model_config = ConfigDict(
        populate_by_name=True, from_attributes=True, use_enum_values=True
    )

class VFolderCreateRequest(BaseSchema):
    name: str
    folder_host: str | None = Field(default=None, alias='host')
    usage_mode: VFolderUsageMode | None = Field(default=VFolderUsageMode.GENERAL.value)
    permission: VFolderPermission | None = Field(default=VFolderPermission.READ_WRITE.value)
    unmanaged_path: str | None = Field(default=None, alias='unmanagedPath')
    group: uuid.UUID | str | None = Field(default=None, alias='group_id')
    cloneable: bool = Field(default=False)

    @field_validator('name')
    @classmethod
    def validate_name(cls, value):
        slug_validator = SlugValidator(allow_dot=True)
        slug_validator.validate(value)
        return value

Separate legacy handlers into Handler, Service, Repository layers

class AbstractVFolderRepository(Protocol):

    async def ensure_host_permission_allowed() -> None:
        ...

    async def get_vfolders(
        user_identity: UserIdentity, 
        allowed_vfolder_types: Sequence[str], 
        extra_vf_conds: list[str]
    ) -> Sequence[Mapping[str, Any]]:
        ...

class AbstractVFolderService(Protocol):

    async def create_vfolder_in_personal(
        user_identity: UserIdentity, 
        keypair: Keypair, 
        group_id: uuid.UUID
    ) -> VFolderAllocationInfo:
        ...
    
    async def create_vfolder_in_group(
        user_identity: UserIdentity, 
        keypair: Keypair, 
        user_group_identity: UserGroupIdentity
    ) -> VFolderAllocationInfo:
        ...

    async def get_vfolders(
            user_identity: UserIdentity
        ) -> CreatedVFolders:
        ...
    
    async def update_vfolder_status(
        vfolder_ids: Sequence[uuid.UUID],
        update_status: VFolderOperationStatus,
        do_log: bool = True,
    ) -> None:
        ...
    
    async def delete_vfolder(
        vfolder_id: uuid.UUID, 
        user_identity: UserIdentity,
        allowed_vfolder_types: Sequence[str],
        resource_policy: Mapping[str, Any],
    ) -> None:
        ...

class AbstractVFolderHandler(Protocol):

    async def create_vfolder(
      self, request: web.Request, params: VFolderCreateRequest
    ) -> web.Response:
        ...

    async def list_folders(
      self, request: web.Request, params: VFolderListRequest
    ) -> web.Response:
        ...
    
    async def delete_vfolder(
      self, request: web.Request, params: VFolderDeleteRequest
    ) -> web.Response:
        ...


class AbstractStorageProxyClient(Protocol):
        
    async def create_vfolder(self):
        ...

    async def delete_vfolder(self):
        ...

class AbstractSharedConfigClient(Protocol):

    async def get_vfolder_types(self):
        ...
    
    async def get_default_host(self):
        ...

    async def get_volume_mount_prefix(self):
        ...

Implementation

class VFolderRepository(AbstractVFolderRepository):
    def __init__(self, db: ExtendedAsyncSAEngine) -> None:
        self.db = db

class VFolderService(AbstractVFolderService):
    def __init__(self, vfolder_repository: VFolderRepository) -> None:
        self.vfolder_repository = vfolder_repository

class StorageProxyService(AbstractStorageProxyService):
    async def create_folder() -> None:
        ...

class VFolderHandler(AbstractVFolderHandler):
    def __init__(
            self,
            vfolder_service: AbstractVFolderService,
            storage_proxy_service: AbstractStorageProxyService
        ):
        self.vfolder_service = vfolder_service
        self.storage_proxy_service = storage_proxy_service

    async def create_vfolder(self, request: web.Request, params: Any) -> web.Response:
        ...

    async def list_folders(self, request: web.Request, params: Any) -> web.Response:
        ...
    
    async def delete_vfolder(self, request: web.Request, params: Any) -> web.Response:
        ...

DI Strategy

The current application manages DB connections through app['root_context'] within the request object. Since existing APIs rely on this approach, modifying this initialization method would 1) require large-scale code modifications and 2) potentially cause unpredictable side effects in existing APIs.

Initially, we adopt a progressive approach to implementing layered architecture. As a first step, we initialize repositories using the DB object from root_context, then use these repositories to initialize services, and store both in the root_context for application-wide use. This strategy allows us to establish the basic structure of layered architecture while minimizing immediate changes to the existing codebase.

The plan is to gradually refactor the root_context implementation in subsequent phases, incrementally expanding the scope of modifications. This phased approach ensures we can maintain system stability while progressively moving towards a more modular and maintainable architecture.

# /manager/api/vfolder.py
def create_app(default_cors_options):
    app = web.Application()
    app["prefix"] = "foldersV2"
    root_ctx: RootContext = app["_root.context"]
    handler: VFolderHandler = VFolderHandler(
        vfolder_service=root_ctx.vfolder_service
        storage_proxy_client=root_ctx.storage_manager
        shared_config_client=root_ctx.shared_config
    )
    cors = aiohttp_cors.setup(app, defaults=default_cors_options)
    add_route = app.router.add_route
    root_resource = cors.add(app.router.add_resource(r""))
    cors.add(root_resource.add_route("POST", handler.create_vfolder))
    cors.add(root_resource.add_route("GET", handler.list_folders))


# /manager/api/context.py
class RootContext(BaseContext):
    # ... 기존 context들
    vfolder_repository: AbstractVFolderRepository
    vfolder_service: AbstractVFolderService


#/manager/api/server.py
@actxmgr
async def vfolfer_component_dependency_ctx(root_ctx: RootContext):
    vfolder_repository: AbstractVFolderRepository = VFolderRepository(db=root_ctx.db)
    vfolder_service: AbstractVFolderService = VFolderService(vfolder_repository=vfolder_repository)
    root_ctx.vfolder_repository = vfolder_repository
    root_ctx.vfolder_service = vfolder_service
    yield

def build_root_app(
    pidx: int,
    local_config: LocalConfig,
    *,
    cleanup_contexts: Optional[Sequence[CleanupContext]] = None,
    subapp_pkgs: Optional[Sequence[str]] = None,
    scheduler_opts: Optional[Mapping[str, Any]] = None,
) -> web.Application:
    ...

    if cleanup_contexts is None:
        cleanup_contexts = [
            manager_status_ctx,
            redis_ctx,
            database_ctx,
            ...,
            vfolfer_component_dependency_ctx
        ]

Impact  

  • Code will be more testable by separating functions into each interest
  • As code size get smaller and less coupled, adding new features or fix bugs will be get easier

Testing Scenarios  

  • Validate correct request parameters structure, return error when invalidate parameters in
  • Verify response format and status code for successful CRUD
  • Mock dependent components(ex. vfolder_service, storagy_proxy_client) to isolate handler testing
@HyeockJinKim HyeockJinKim changed the title Implement CRUD handlers for manager Add Interface for vFolder CRUD handlers in Manager Jan 15, 2025
@HyeockJinKim HyeockJinKim changed the title Add Interface for vFolder CRUD handlers in Manager Add a new handler with an empty interface to handle VFolder CRUD API in manager Jan 15, 2025
@HyeockJinKim HyeockJinKim changed the title Add a new handler with an empty interface to handle VFolder CRUD API in manager Add the skeleton interface of VFolder CRUD APIs using the new layered architecture in Manager Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants