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

feat: Accept multiple paths in the vfolder mkdir API #1803

Merged
merged 26 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
4df4959
feature: now accepts multiple mkdir
mirageoasis Dec 21, 2023
2aecd92
feature: now can use multiple mkdir in single http request
mirageoasis Dec 21, 2023
3ece4e9
fixed: mypy complains about type check
mirageoasis Dec 21, 2023
9813fa1
fixed: fixed according to feedback
mirageoasis Dec 22, 2023
9c3a5fe
fixed: validators now can check type of t.Or()
mirageoasis Dec 22, 2023
16d2d07
feature: now check handling error and fixed docs accordingly
mirageoasis Dec 23, 2023
bbce4fa
feature: refined error handling
mirageoasis Dec 26, 2023
713463a
feature: added cli success and failure tasks
mirageoasis Dec 28, 2023
1fee3bb
refactor: list typing deprecated
mirageoasis Dec 29, 2023
15a2af0
refactor: list typing deprecated
mirageoasis Dec 29, 2023
a7d6e6e
refactor: list typing deprecated
mirageoasis Dec 29, 2023
02c6e27
fixed: fixed error message represenataion and description of cli
mirageoasis Jan 30, 2024
4b06edd
Merge branch 'main' into feature/accept-multiple-mkdir
achimnol Feb 13, 2024
e8006d8
docs: modify helper message for mkdir command in vfolder
SangwonYoon Feb 13, 2024
1a9da48
modify helper message for mkdir command in vfolder
SangwonYoon Feb 13, 2024
acd812c
refactor: Let's prefer modern Python unless it makes backport harder
achimnol Feb 13, 2024
f6bf2df
refactor: Make multi-result response handling consistent
achimnol Feb 13, 2024
3b6eac2
fix: Unwrap the result set from the API response field
achimnol Feb 13, 2024
fa1e7db
fix: Use 422 instead of 500 to indicate semantic failures
achimnol Feb 13, 2024
7957bf4
refactor: Let it keep the responsibility of storage_manager
achimnol Feb 13, 2024
93aeb90
Merge branch 'main' into feature/accept-multiple-mkdir
achimnol Feb 27, 2024
3625a56
fix: oops
achimnol Feb 27, 2024
b02013a
fix: Simplify the failure list output
achimnol Feb 27, 2024
92849ac
feat: Add print_result_set() to the output framework and use it
achimnol Feb 27, 2024
8ad302f
refactor: Provide a little richer info about the API failure response
achimnol Feb 27, 2024
08a80ca
fix: Remove too much nesting of error data output in CLI
achimnol Feb 27, 2024
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
1 change: 1 addition & 0 deletions changes/1803.feature.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for multi directory mkdir by fixing cli to accept multiple arguments and by adding list type annotation to accept multiple directories
10 changes: 6 additions & 4 deletions src/ai/backend/client/cli/vfolder.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ def cp(filenames):

@vfolder.command()
@click.argument("name", type=str)
@click.argument("path", type=str)
@click.argument("paths", type=str, nargs=-1)
achimnol marked this conversation as resolved.
Show resolved Hide resolved
@click.option(
"-p",
"--parents",
Expand All @@ -445,17 +445,19 @@ def cp(filenames):
is_flag=True,
help="Skip an error caused by file not found",
)
def mkdir(name, path, parents, exist_ok):
def mkdir(name, paths, parents, exist_ok):
"""Create an empty directory in the virtual folder.

\b
NAME: Name of a virtual folder.
PATH: The name or path of directory. Parent directories are created automatically
PATH: Mutliple arguments of name or path of directory seperated by space. Parent directories are created automatically
if they do not exist.

Example: backend.ai vfolder mkdir my_vfolder "dir1" "dir2" "dir3"
"""
with Session() as session:
try:
session.VFolder(name).mkdir(path, parents=parents, exist_ok=exist_ok)
session.VFolder(name).mkdir(paths, parents=parents, exist_ok=exist_ok)
print_done("Done.")
except Exception as e:
print_error(e)
Expand Down
8 changes: 4 additions & 4 deletions src/ai/backend/client/func/vfolder.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import asyncio
from pathlib import Path
from typing import Dict, Mapping, Optional, Sequence, Union
from typing import Dict, List, Mapping, Optional, Sequence, Union

import aiohttp
import janus
Expand Down Expand Up @@ -475,7 +475,7 @@ async def _upload_recursively(
if path.is_file():
file_list.append(path)
else:
await self._mkdir(path.relative_to(base_path))
await self._mkdir([path.relative_to(base_path)])
dir_list.append(path)
await self._upload_files(file_list, basedir, dst_dir, chunk_size, address_map)
for dir in dir_list:
Expand Down Expand Up @@ -506,7 +506,7 @@ async def upload(

async def _mkdir(
self,
path: Union[str, Path],
path: str | Path | List[str | Path],
achimnol marked this conversation as resolved.
Show resolved Hide resolved
parents: Optional[bool] = False,
exist_ok: Optional[bool] = False,
) -> str:
Expand All @@ -522,7 +522,7 @@ async def _mkdir(
@api_function
async def mkdir(
self,
path: Union[str, Path],
path: str | Path | List[str | Path],
achimnol marked this conversation as resolved.
Show resolved Hide resolved
parents: Optional[bool] = False,
exist_ok: Optional[bool] = False,
) -> str:
Expand Down
6 changes: 5 additions & 1 deletion src/ai/backend/common/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,11 @@ def __init__(
self._relative_only = relative_only

def check_and_return(self, value: Any) -> _PurePath:
p = _PurePath(value)
try:
p = _PurePath(value)
except (TypeError, ValueError):
self._failure("cannot parse value as a path", value=value)

if self._relative_only and p.is_absolute():
self._failure("expected relative path but the value is absolute", value=value)
if self._base_path is not None:
Expand Down
4 changes: 2 additions & 2 deletions src/ai/backend/manager/api/vfolder.py
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,7 @@ async def update_vfolder_options(
@vfolder_permission_required(VFolderPermission.READ_WRITE)
@check_api_params(
t.Dict({
t.Key("path"): t.String,
t.Key("path"): t.String | t.List(t.String),
t.Key("parents", default=True): t.ToBool,
t.Key("exist_ok", default=False): t.ToBool,
})
Expand All @@ -1239,7 +1239,7 @@ async def mkdir(request: web.Request, params: Any, row: VFolderRow) -> web.Respo
folder_name = request.match_info["name"]
access_key = request["keypair"]["access_key"]
log.info(
"VFOLDER.MKDIR (email:{}, ak:{}, vf:{}, path:{})",
"VFOLDER.MKDIR (email:{}, ak:{}, vf:{}, paths:{})",
request["user"]["email"],
access_key,
folder_name,
Expand Down
2 changes: 1 addition & 1 deletion src/ai/backend/manager/models/vfolder.py
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ async def prepare_vfolder_mounts(
params={
"volume": storage_manager.split_host(vfolder["host"])[1],
"vfid": str(VFolderID(vfolder["quota_scope_id"], vfolder["id"])),
"relpath": str(user_scope.user_uuid.hex),
"relpaths": [str(user_scope.user_uuid.hex)],
"exist_ok": True,
},
):
Expand Down
59 changes: 51 additions & 8 deletions src/ai/backend/storage/api/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

from __future__ import annotations

import asyncio
import errno
import json
import logging
import os
Expand Down Expand Up @@ -755,7 +757,7 @@ async def mkdir(request: web.Request) -> web.Response:
class Params(TypedDict):
volume: str
vfid: VFolderID
relpath: PurePosixPath
relpath: PurePosixPath | List[PurePosixPath]
mirageoasis marked this conversation as resolved.
Show resolved Hide resolved
parents: bool
exist_ok: bool

Expand All @@ -767,7 +769,8 @@ class Params(TypedDict):
{
t.Key("volume"): t.String(),
t.Key("vfid"): tx.VFolderID(),
t.Key("relpath"): tx.PurePath(relative_only=True),
t.Key("relpath"): tx.PurePath(relative_only=True)
| t.List(tx.PurePath(relative_only=True)),
t.Key("parents", default=True): t.ToBool,
t.Key("exist_ok", default=False): t.ToBool,
},
Expand All @@ -776,14 +779,54 @@ class Params(TypedDict):
) as params:
await log_manager_api_entry(log, "mkdir", params)
ctx: RootContext = request.app["ctx"]
vfid = params["vfid"]
parents = params["parents"]
exist_ok = params["exist_ok"]
relpath = params["relpath"]
relpaths = relpath if isinstance(relpath, list) else [relpath]
error_file_list: List[str] = []
success_file_list: List[str] = []

async with ctx.get_volume(params["volume"]) as volume:
with handle_fs_errors(volume, params["vfid"]):
await volume.mkdir(
params["vfid"],
params["relpath"],
parents=params["parents"],
exist_ok=params["exist_ok"],
mkdir_tasks = [
volume.mkdir(vfid, rpath, parents=parents, exist_ok=exist_ok) for rpath in relpaths
]
result_group = await asyncio.gather(*mkdir_tasks, return_exceptions=True)

# if result group is all false throw error.
if all([isinstance(res, FileExistsError) for res in result_group]):
raise web.HTTPBadRequest(
body=json.dumps(
{
"msg": "FileExistsError None of directories created because they already exist",
"errno": errno.EEXIST,
},
),
content_type="application/json",
)

if any([isinstance(res, BaseException) for res in result_group]):
for relpath, result_or_exception in zip(relpaths, result_group):
if isinstance(result_or_exception, BaseException):
log.error(
"{}",
repr(result_or_exception),
exc_info=result_or_exception,
)
error_file_list.append(f"[Errno {errno.EEXIST}] File exists: {relpath}")
else:
success_file_list.append(str(relpath))

raise web.HTTPBadRequest(
body=json.dumps(
{
"error_file_list": error_file_list,
"success_file_list": success_file_list,
},
),
content_type="application/json",
)

return web.Response(status=204)


Expand Down
Loading