Skip to content

Commit

Permalink
fix: rename Image Filter to Image Type (#2555)
Browse files Browse the repository at this point in the history
  • Loading branch information
kyujin-cho authored Aug 5, 2024
1 parent 65e02e4 commit 3ff5cf9
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 29 deletions.
1 change: 1 addition & 0 deletions changes/2555.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Rename `images.image_filters` GQL Query argument to `images.image_types`
9 changes: 7 additions & 2 deletions src/ai/backend/manager/api/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,17 @@ type Queries {
Added in 19.09.0. If it is specified, fetch images installed on at least one agent.
"""
is_installed: Boolean
is_operation: Boolean @deprecated(reason: "Deprecated since 24.03.4. This field is ignored if `image_filters` is specified and is not null.")
is_operation: Boolean @deprecated(reason: "Deprecated since 24.03.4. This field is ignored if `load_filters` is specified and is not null.")

"""
Added in 24.03.8. Allowed values are: [general, operational, customized]. When superuser queries with `customized` option set the resolver will return every customized images (including those not owned by callee). To resolve images owned by user only call `customized_images`.
"""
load_filters: [String] = null

"""
Added in 24.03.4. Allowed values are: [general, operational, customized]. When superuser queries with `customized` option set the resolver will return every customized images (including those not owned by caller). To list the owned images only call `customized_images`.
"""
image_filters: [String] = null
image_filters: [String] = null @deprecated(reason: "Deprecated since 24.03.8. Use `load_filters` instead.")
): [Image]

"""Added in 24.03.1"""
Expand Down
38 changes: 22 additions & 16 deletions src/ai/backend/manager/models/gql.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,17 @@ class Queries(graphene.ObjectType):
description="Added in 19.09.0. If it is specified, fetch images installed on at least one agent."
),
is_operation=graphene.Boolean(
deprecation_reason="Deprecated since 24.03.4. This field is ignored if `image_filters` is specified and is not null."
deprecation_reason="Deprecated since 24.03.4. This field is ignored if `load_filters` is specified and is not null."
),
load_filters=graphene.List(
graphene.String,
default_value=None,
description=f"Added in 24.03.8. Allowed values are: [{', '.join([f.value for f in PublicImageLoadFilter])}]. When superuser queries with `customized` option set the resolver will return every customized images (including those not owned by callee). To resolve images owned by user only call `customized_images`.",
),
image_filters=graphene.List(
graphene.String,
default_value=None,
deprecation_reason="Deprecated since 24.03.8. Use `load_filters` instead.",
description=f"Added in 24.03.4. Allowed values are: [{', '.join([f.value for f in PublicImageLoadFilter])}]. When superuser queries with `customized` option set the resolver will return every customized images (including those not owned by caller). To list the owned images only call `customized_images`.",
),
)
Expand Down Expand Up @@ -1106,7 +1112,7 @@ async def resolve_customized_images(
client_domain = ctx.user["domain_name"]
items = await Image.load_all(
ctx,
filters=set((ImageLoadFilter.CUSTOMIZED,)),
types=set((ImageLoadFilter.CUSTOMIZED,)),
)
if client_role == UserRole.SUPERADMIN:
pass
Expand All @@ -1133,38 +1139,38 @@ async def resolve_images(
*,
is_installed: bool | None = None,
is_operation=False,
load_filters: list[str] | None = None,
image_filters: list[str] | None = None,
) -> Sequence[Image]:
ctx: GraphQueryContext = info.context
client_role = ctx.user["role"]
client_domain = ctx.user["domain_name"]
image_load_filters: set[ImageLoadFilter] = set()
if image_filters is not None:
image_load_types: set[ImageLoadFilter] = set()
_types = load_filters or image_filters
if _types is not None:
try:
_filters: list[PublicImageLoadFilter] = [
PublicImageLoadFilter(f) for f in image_filters
]
_filters: list[PublicImageLoadFilter] = [PublicImageLoadFilter(f) for f in _types]
except ValueError as e:
allowed_filter_values = ", ".join([f.value for f in PublicImageLoadFilter])
raise InvalidAPIParameters(
f"{e}. All elements of `image_filters` should be one of ({allowed_filter_values})"
f"{e}. All elements of `load_filters` should be one of ({allowed_filter_values})"
)
image_load_filters.update([ImageLoadFilter(f) for f in _filters])
image_load_types.update([ImageLoadFilter(f) for f in _filters])
if (
client_role == UserRole.SUPERADMIN
and ImageLoadFilter.CUSTOMIZED in image_load_filters
and ImageLoadFilter.CUSTOMIZED in image_load_types
):
image_load_filters.remove(ImageLoadFilter.CUSTOMIZED)
image_load_filters.add(ImageLoadFilter.CUSTOMIZED_GLOBAL)
image_load_types.remove(ImageLoadFilter.CUSTOMIZED)
image_load_types.add(ImageLoadFilter.CUSTOMIZED_GLOBAL)
else:
image_load_filters.add(ImageLoadFilter.CUSTOMIZED)
image_load_filters.add(ImageLoadFilter.GENERAL)
image_load_types.add(ImageLoadFilter.CUSTOMIZED)
image_load_types.add(ImageLoadFilter.GENERAL)
if is_operation is None:
# I know this logic is quite contradicts to the parameter name,
# but to conform with previous implementation...
image_load_filters.add(ImageLoadFilter.OPERATIONAL)
image_load_types.add(ImageLoadFilter.OPERATIONAL)

items = await Image.load_all(ctx, filters=image_load_filters)
items = await Image.load_all(ctx, types=image_load_types)
if client_role == UserRole.SUPERADMIN:
pass
elif client_role in (UserRole.ADMIN, UserRole.USER):
Expand Down
26 changes: 15 additions & 11 deletions src/ai/backend/manager/models/image.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@


class PublicImageLoadFilter(enum.StrEnum):
"""Shorthand of `ImageLoadFilter` enum with `CUSTOMIZED_GLOBAL` removed (as it is not intended for API input)."""

GENERAL = "general"
"""Include general purpose images."""
OPERATIONAL = "operational"
Expand All @@ -94,6 +96,8 @@ class PublicImageLoadFilter(enum.StrEnum):


class ImageLoadFilter(enum.StrEnum):
"""Enum describing kind of a "search preset" when loading Image data via GQL. Not intended for declaring attributes of image data itself."""

GENERAL = "general"
"""Include general purpose images."""
OPERATIONAL = "operational"
Expand Down Expand Up @@ -676,12 +680,12 @@ async def load_all(
cls,
ctx: GraphQueryContext,
*,
filters: set[ImageLoadFilter] = set(),
types: set[ImageLoadFilter] = set(),
) -> Sequence[Image]:
async with ctx.db.begin_readonly_session() as session:
rows = await ImageRow.list(session, load_aliases=True)
items: list[Image] = [
item async for item in cls.bulk_load(ctx, rows) if item.matches_filter(ctx, filters)
item async for item in cls.bulk_load(ctx, rows) if item.matches_type(ctx, types)
]

return items
Expand Down Expand Up @@ -709,38 +713,38 @@ async def filter_allowed(

return filtered_items

def matches_filter(
def matches_type(
self,
ctx: GraphQueryContext,
filters: set[ImageLoadFilter],
load_filters: set[ImageLoadFilter],
) -> bool:
"""
Determine if the image is filtered according to the `filters` parameter.
Determine if the image is filtered according to the `load_filters` parameter.
"""
user_role = ctx.user["role"]

# If the image filtered by any of its labels, return False early.
# If the image is not filtered and is determiend to be valid by any of its labels, `is_valid = True`.
is_valid = ImageLoadFilter.GENERAL in filters
is_valid = ImageLoadFilter.GENERAL in load_filters
for label in self.labels:
match label.key:
case "ai.backend.features" if "operation" in label.value:
if ImageLoadFilter.OPERATIONAL in filters:
if ImageLoadFilter.OPERATIONAL in load_filters:
is_valid = True
else:
return False
case "ai.backend.customized-image.owner":
if (
ImageLoadFilter.CUSTOMIZED not in filters
and ImageLoadFilter.CUSTOMIZED_GLOBAL not in filters
ImageLoadFilter.CUSTOMIZED not in load_filters
and ImageLoadFilter.CUSTOMIZED_GLOBAL not in load_filters
):
return False
if ImageLoadFilter.CUSTOMIZED in filters:
if ImageLoadFilter.CUSTOMIZED in load_filters:
if label.value == f"user:{ctx.user['uuid']}":
is_valid = True
else:
return False
if ImageLoadFilter.CUSTOMIZED_GLOBAL in filters:
if ImageLoadFilter.CUSTOMIZED_GLOBAL in load_filters:
if user_role == UserRole.SUPERADMIN:
is_valid = True
else:
Expand Down

0 comments on commit 3ff5cf9

Please sign in to comment.