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

mirageoasis · 2023-12-21T08:07:29Z

This PR adds an extra feature to specify multiple directories in the vfolder mkdir API to create them via a single API call.

Allow passing multiple relative paths of directores to create via nargs in click.option
Add '-p' option to auto-create parent directories recursively

The feature itself is a relatively small addition, but this PR demonstrates how we are going to design and implement multi-result (or batch) APIs.

Introduce common.types.{ItemResult,ResultSet} to distinguish success/failed results, with optional error messages attached to each item.
- Now the client SDK's output framework also has print_result_set() API.
The HTTP response codes will be segregated as follows:
- 200: All success
- 207: Partial success/failure (from WebDAV)
- 422: All failure (semantic failure, not server error) (from WebDAV)

Examples

Partial success (console output):

Partial success (JSON output):

All failed (JSON output):

fregataa

Good!
I left some comments

src/ai/backend/client/cli/vfolder.py

src/ai/backend/client/func/vfolder.py

src/ai/backend/manager/api/vfolder.py

src/ai/backend/storage/api/manager.py

fregataa

some more comments!

src/ai/backend/client/func/vfolder.py

changes/1803.feature.md

src/ai/backend/storage/api/manager.py

src/ai/backend/client/cli/vfolder.py

fregataa

good!
I checked the functionality and it works well.
I left a few comment about typing

src/ai/backend/storage/api/manager.py

mirageoasis · 2024-01-03T10:01:04Z

@achimnol
finished with issue #409 !

src/ai/backend/storage/api/manager.py

src/ai/backend/client/cli/vfolder.py

src/ai/backend/manager/api/vfolder.py

- Introduce `common.types.{ItemResult,ResultSet}` for consistent typing and structuring of future multi-result APIs

- Also limit the maximum number of paths to create concurrently (50). - Expose non-5xx storage-proxy responses without wrapping them as "InvalidAPIParameters" which may be misleading for semantic failures.

mirageoasis · 2024-02-27T14:07:11Z

Totally different api from what I first wrote! amazing!
More fined grained reports!

feature: now accepts multiple mkdir

4df4959

github-actions bot assigned mirageoasis Dec 21, 2023

github-actions bot added comp:client Related to Client component size:S 10~30 LoC labels Dec 21, 2023

mirageoasis requested a review from fregataa December 21, 2023 08:08

mirageoasis added 2 commits December 21, 2023 22:57

feature: now can use multiple mkdir in single http request

2aecd92

fixed: mypy complains about type check

3ece4e9

github-actions bot added comp:manager Related to Manager component comp:storage-proxy Related to Storage proxy component size:M 30~100 LoC and removed size:S 10~30 LoC labels Dec 21, 2023

mirageoasis marked this pull request as ready for review December 21, 2023 14:31

fregataa reviewed Dec 22, 2023

View reviewed changes

fixed: fixed according to feedback

9813fa1

mirageoasis requested a review from fregataa December 22, 2023 06:42

fregataa reviewed Dec 22, 2023

View reviewed changes

src/ai/backend/client/func/vfolder.py Outdated Show resolved Hide resolved

src/ai/backend/client/func/vfolder.py Outdated Show resolved Hide resolved

changes/1803.feature.md Outdated Show resolved Hide resolved

src/ai/backend/storage/api/manager.py Outdated Show resolved Hide resolved

mirageoasis added 2 commits December 22, 2023 16:55

fixed: validators now can check type of t.Or()

9c3a5fe

feature: now check handling error and fixed docs accordingly

16d2d07

mirageoasis requested a review from fregataa December 23, 2023 22:24

fregataa reviewed Dec 26, 2023

View reviewed changes

src/ai/backend/storage/api/manager.py Outdated Show resolved Hide resolved

src/ai/backend/storage/api/manager.py Outdated Show resolved Hide resolved

feature: refined error handling

bbce4fa

mirageoasis commented Dec 26, 2023

View reviewed changes

src/ai/backend/client/cli/vfolder.py Show resolved Hide resolved

feature: added cli success and failure tasks

713463a

github-actions bot added size:L 100~500 LoC and removed size:M 30~100 LoC labels Dec 28, 2023

mirageoasis requested review from achimnol and fregataa December 28, 2023 15:18

fregataa reviewed Dec 29, 2023

View reviewed changes

src/ai/backend/storage/api/manager.py Outdated Show resolved Hide resolved

src/ai/backend/storage/api/manager.py Outdated Show resolved Hide resolved

mirageoasis added 2 commits December 29, 2023 15:26

refactor: list typing deprecated

1fee3bb

refactor: list typing deprecated

15a2af0

achimnol added this to the 24.03 milestone Jan 9, 2024

mirageoasis changed the title ~~feature: now accepts multiple mkdir~~ feat: now accepts multiple mkdir Jan 22, 2024

achimnol requested changes Jan 30, 2024

View reviewed changes

src/ai/backend/storage/api/manager.py Outdated Show resolved Hide resolved

src/ai/backend/client/cli/vfolder.py Outdated Show resolved Hide resolved

achimnol changed the title ~~feat: now accepts multiple mkdir~~ feat: Accept multiple paths in the vfolder mkdir API Jan 30, 2024

mirageoasis and others added 2 commits January 30, 2024 22:06

fixed: fixed error message represenataion and description of cli

02c6e27

Merge branch 'main' into feature/accept-multiple-mkdir

4b06edd

achimnol requested changes Feb 13, 2024

View reviewed changes

src/ai/backend/manager/api/vfolder.py Outdated Show resolved Hide resolved

SangwonYoon and others added 12 commits February 13, 2024 04:47

docs: modify helper message for mkdir command in vfolder

e8006d8

modify helper message for mkdir command in vfolder

1a9da48

refactor: Let's prefer modern Python unless it makes backport harder

acd812c

refactor: Make multi-result response handling consistent

f6bf2df

- Introduce `common.types.{ItemResult,ResultSet}` for consistent typing and structuring of future multi-result APIs

fix: Unwrap the result set from the API response field

3b6eac2

fix: Use 422 instead of 500 to indicate semantic failures

fa1e7db

- Also limit the maximum number of paths to create concurrently (50). - Expose non-5xx storage-proxy responses without wrapping them as "InvalidAPIParameters" which may be misleading for semantic failures.

refactor: Let it keep the responsibility of storage_manager

7957bf4

Merge branch 'main' into feature/accept-multiple-mkdir

93aeb90

fix: oops

3625a56

fix: Simplify the failure list output

b02013a

feat: Add print_result_set() to the output framework and use it

92849ac

refactor: Provide a little richer info about the API failure response

8ad302f

achimnol enabled auto-merge February 27, 2024 14:01

achimnol disabled auto-merge February 27, 2024 14:02

fix: Remove too much nesting of error data output in CLI

08a80ca

achimnol enabled auto-merge February 27, 2024 14:05

achimnol added this pull request to the merge queue Feb 27, 2024

Merged via the queue into main with commit c630fb7 Feb 27, 2024
30 checks passed

achimnol deleted the feature/accept-multiple-mkdir branch February 27, 2024 14:12

fregataa mentioned this pull request May 13, 2024

fix: Wrong success check of vfolder cli integration test #2140

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

mirageoasis commented Dec 21, 2023 •

edited by achimnol

Loading

fregataa left a comment

fregataa left a comment

fregataa left a comment

mirageoasis commented Jan 3, 2024

mirageoasis commented Feb 27, 2024

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

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

Conversation

mirageoasis commented Dec 21, 2023 • edited by achimnol Loading

Examples

fregataa left a comment

Choose a reason for hiding this comment

fregataa left a comment

Choose a reason for hiding this comment

fregataa left a comment

Choose a reason for hiding this comment

mirageoasis commented Jan 3, 2024

mirageoasis commented Feb 27, 2024

mirageoasis commented Dec 21, 2023 •

edited by achimnol

Loading