Skip to content

Commit

Permalink
fix: Make all CLI mutation commands aware of undefined
Browse files Browse the repository at this point in the history
- Fix several module's `__all__` variable definition
- Rename `--allwed-vfolder-hosts` to `--vfolder-host-perms` where possible.
  This may be breaking change for Client SDK functional wrapper users,
  but we keep the legacy the backward compatibility for CLI users.
  • Loading branch information
achimnol committed Nov 5, 2023
1 parent b3c483d commit b8428c8
Show file tree
Hide file tree
Showing 11 changed files with 358 additions and 191 deletions.
39 changes: 22 additions & 17 deletions src/ai/backend/client/cli/admin/domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,19 +70,21 @@ def list(ctx: CLIContext) -> None:
help="Set total resource slots as a JSON string.",
)
@click.option(
"--allowed-vfolder-hosts",
"--vfolder-host-perms",
"--vfolder-host-permissions",
"--vfhost-perms",
type=str,
default="{}",
"--allowed-vfolder-hosts", # legacy name
type=OptionalType(str),
default=undefined,
help=(
"Allowed virtual folder hosts and permissions for them. It must be JSON string (e.g:"
' --allowed-vfolder-hosts=\'{"HOST_NAME": ["create-vfolder", "modify-vfolder"]}\')'
' --vfolder-host-perms=\'{"HOST_NAME": ["create-vfolder", "modify-vfolder"]}\')'
),
)
@click.option(
"--allowed-docker-registries",
type=CommaSeparatedListType(),
type=OptionalType(CommaSeparatedListType),
default=undefined,
help="Allowed docker registries.",
)
def add(
Expand All @@ -91,9 +93,9 @@ def add(
description: str,
inactive: bool,
total_resource_slots: str,
allowed_vfolder_hosts: str,
allowed_docker_registries: Sequence[str],
):
vfolder_host_perms: str | Undefined,
allowed_docker_registries: Sequence[str] | Undefined,
) -> None:
"""
Add a new domain.
Expand All @@ -106,7 +108,7 @@ def add(
description=description,
is_active=not inactive,
total_resource_slots=total_resource_slots,
allowed_vfolder_hosts=allowed_vfolder_hosts,
vfolder_host_perms=vfolder_host_perms,
allowed_docker_registries=allowed_docker_registries,
)
except Exception as e:
Expand Down Expand Up @@ -157,12 +159,15 @@ def add(
help="Update total resource slots.",
)
@click.option(
"--allowed-vfolder-hosts",
"--vfolder-host-perms",
"--vfolder-host-permissions",
"--vfhost-perms",
"--allowed-vfolder-hosts", # legacy name
type=OptionalType(str),
default=undefined,
help=(
"Allowed virtual folder hosts. It must be JSON string (e.g:"
' --allowed-vfolder-hosts=\'{"HOST_NAME": ["create-vfolder", "modify-vfolder"]}\')'
"Allowed virtual folder hosts and permissions for them. It must be JSON string (e.g:"
' --vfolder-host-perms=\'{"HOST_NAME": ["create-vfolder", "modify-vfolder"]}\')'
),
)
@click.option(
Expand All @@ -178,9 +183,9 @@ def update(
description: str | Undefined,
is_active: bool | Undefined,
total_resource_slots: str | Undefined,
allowed_vfolder_hosts: str | Undefined,
vfolder_host_perms: str | Undefined,
allowed_docker_registries: Sequence[str] | Undefined,
):
) -> None:
"""
Update an existing domain.
Expand All @@ -194,7 +199,7 @@ def update(
description=description,
is_active=is_active,
total_resource_slots=total_resource_slots,
allowed_vfolder_hosts=allowed_vfolder_hosts,
vfolder_host_perms=vfolder_host_perms,
allowed_docker_registries=allowed_docker_registries,
)
except Exception as e:
Expand Down Expand Up @@ -222,7 +227,7 @@ def update(
@domain.command()
@pass_ctx_obj
@click.argument("name", type=str, metavar="NAME")
def delete(ctx: CLIContext, name):
def delete(ctx: CLIContext, name: str) -> None:
"""
Inactive an existing domain.
Expand Down Expand Up @@ -256,7 +261,7 @@ def delete(ctx: CLIContext, name):
@domain.command()
@pass_ctx_obj
@click.argument("name", type=str, metavar="NAME")
def purge(ctx: CLIContext, name):
def purge(ctx: CLIContext, name: str) -> None:
"""
Delete an existing domain.
Expand Down
94 changes: 66 additions & 28 deletions src/ai/backend/client/cli/admin/group.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import sys
import uuid
from typing import Sequence

import click

from ai.backend.cli.interaction import ask_yn
from ai.backend.cli.types import ExitCode
from ai.backend.client.func.group import _default_detail_fields, _default_list_fields
from ai.backend.client.session import Session
from ai.backend.client.cli.params import OptionalType

from ...func.group import _default_detail_fields, _default_list_fields
from ...session import Session
from ...types import Undefined, undefined
from ..extensions import pass_ctx_obj
from ..pretty import print_info
from ..types import CLIContext
Expand Down Expand Up @@ -89,23 +92,26 @@ def list(ctx: CLIContext, domain_name) -> None:
@click.option("-i", "--inactive", is_flag=True, help="New group will be inactive.")
@click.option("--total-resource-slots", type=str, default="{}", help="Set total resource slots.")
@click.option(
"--allowed-vfolder-hosts",
"--vfolder-host-perms",
"--vfolder-host-permissions",
"--vfhost-perms",
"--allowed-vfolder-hosts", # legacy name
type=str,
default="{}",
help=(
"Allowed virtual folder hosts. It must be JSON string (e.g:"
' --allowed-vfolder-hosts=\'{"HOST_NAME": ["create-vfolder", "modify-vfolder"]}\')'
"Allowed virtual folder hosts and permissions for them. It must be JSON string (e.g:"
' --vfolder-host-perms=\'{"HOST_NAME": ["create-vfolder", "modify-vfolder"]}\')'
),
)
def add(
ctx: CLIContext,
domain_name,
name,
description,
inactive,
total_resource_slots,
allowed_vfolder_hosts,
):
domain_name: str,
name: str,
description: str,
inactive: bool,
total_resource_slots: str, # JSON string
vfolder_host_perms: str, # JSON string
) -> None:
"""
Add new group. A group must belong to a domain, so DOMAIN_NAME should be provided.
Expand All @@ -121,7 +127,7 @@ def add(
description=description,
is_active=not inactive,
total_resource_slots=total_resource_slots,
allowed_vfolder_hosts=allowed_vfolder_hosts,
allowed_vfolder_hosts=vfolder_host_perms,
)
except Exception as e:
ctx.output.print_mutation_error(
Expand All @@ -146,21 +152,53 @@ def add(
@group.command()
@pass_ctx_obj
@click.argument("gid", type=str, metavar="GROUP_ID")
@click.option("-n", "--name", type=str, help="New name of the group")
@click.option("-d", "--description", type=str, help="Description of the group")
@click.option("--is-active", type=bool, help="Set group inactive.")
@click.option("--total-resource-slots", type=str, help="Update total resource slots.")
@click.option(
"--allowed-vfolder-hosts",
type=str,
"-n",
"--name",
type=OptionalType(str),
default=undefined,
help="New name of the group",
)
@click.option(
"-d",
"--description",
type=OptionalType(str),
default=undefined,
help="Description of the group",
)
@click.option(
"--is-active",
type=OptionalType(bool),
default=undefined,
help="Set group inactive.",
)
@click.option(
"--total-resource-slots",
type=OptionalType(str),
default=undefined,
help="Update total resource slots.",
)
@click.option(
"--vfolder-host-perms",
"--vfolder-host-permissions",
"--vfhost-perms",
"--allowed-vfolder-hosts", # legacy name
type=OptionalType(str),
default=undefined,
help=(
"Allowed virtual folder hosts. It must be JSON string (e.g:"
' --allowed-vfolder-hosts=\'{"HOST_NAME": ["create-vfolder", "modify-vfolder"]}\')'
"Allowed virtual folder hosts and permissions for them. It must be JSON string (e.g:"
' --vfolder-host-perms=\'{"HOST_NAME": ["create-vfolder", "modify-vfolder"]}\')'
),
)
def update(
ctx: CLIContext, gid, name, description, is_active, total_resource_slots, allowed_vfolder_hosts
):
ctx: CLIContext,
gid: str | Undefined,
name: str | Undefined,
description: str | Undefined,
is_active: bool | Undefined,
total_resource_slots: str | Undefined, # JSON string
vfolder_host_perms: str | Undefined, # JSON string
) -> None:
"""
Update an existing group. Domain name is not necessary since group ID is unique.
Expand All @@ -174,7 +212,7 @@ def update(
description=description,
is_active=is_active,
total_resource_slots=total_resource_slots,
allowed_vfolder_hosts=allowed_vfolder_hosts,
allowed_vfolder_hosts=vfolder_host_perms,
)
except Exception as e:
ctx.output.print_mutation_error(
Expand All @@ -201,7 +239,7 @@ def update(
@group.command()
@pass_ctx_obj
@click.argument("gid", type=str, metavar="GROUP_ID")
def delete(ctx: CLIContext, gid):
def delete(ctx: CLIContext, gid: str) -> None:
"""
Inactivates the existing group. Does not actually delete it for safety.
Expand Down Expand Up @@ -235,7 +273,7 @@ def delete(ctx: CLIContext, gid):
@group.command()
@pass_ctx_obj
@click.argument("gid", type=str, metavar="GROUP_ID")
def purge(ctx: CLIContext, gid):
def purge(ctx: CLIContext, gid: str) -> None:
"""
Delete the existing group. This action cannot be undone.
Expand Down Expand Up @@ -273,7 +311,7 @@ def purge(ctx: CLIContext, gid):
@pass_ctx_obj
@click.argument("gid", type=str, metavar="GROUP_ID")
@click.argument("user_uuids", type=str, metavar="USER_UUIDS", nargs=-1)
def add_users(ctx: CLIContext, gid, user_uuids):
def add_users(ctx: CLIContext, gid: str, user_uuids: Sequence[str]) -> None:
"""
Add users to a group.
Expand Down Expand Up @@ -310,7 +348,7 @@ def add_users(ctx: CLIContext, gid, user_uuids):
@pass_ctx_obj
@click.argument("gid", type=str, metavar="GROUP_ID")
@click.argument("user_uuids", type=str, metavar="USER_UUIDS", nargs=-1)
def remove_users(ctx: CLIContext, gid, user_uuids):
def remove_users(ctx: CLIContext, gid: str, user_uuids: Sequence[str]) -> None:
"""
Remove users from a group.
Expand Down
Loading

0 comments on commit b8428c8

Please sign in to comment.