refactor: re-define vfolder delete status #1892

fregataa · 2024-02-06T11:16:28Z

refs #767
related to lablup/backend.ai-webui#2204

!! MUST

do alembic migration upgrade. ./py -m alembic upgrade head
DON'T FORGET to stamp. ./py -m alembic stamp head
You must stamp even when you revert the DB schema to main. ./py -m alembic downgrade -1, ./py -m alembic stamp a5319bfc7d7c

Updates

Add delete-pending, delete-error vfolder status
Set delete-pending status as a "trash-bin" stataus
Set delete-complete status as vfolder hard delete in storage proxy
Replace deleted-complete to delete-pending
Rename recover API to restore. Because recover is a more appropriate term when you want to retrieve a file or data after you lose it, and restore is more often used when you want to return to a certain point in time or state.

Updated/Added API

DELETE /folders now accepts any of vfolder_id, vfolderId, id parameter and the response contains success message

curl --location --request DELETE 'MANAGER_ENDPOINT/folders' \
--header 'Content-Type: application/json' \
--data '{
    "vfolder_id": "VFOLDER_ID"
    # or "vfolderId", "id"
}'

POST /folders/restore-from-trash-bin restores soft-deleted vfolders(aka trash-bin) into ready state.

curl --location 'MANAGER_ENDPOINT/folders/restore-from-trash-bin' \
--header 'Content-Type: application/json' \
--data '{
    "vfolder_id": "VFOLDER_ID"
}'

POST /folders/delete-from-trash-bin permanently deletes vfolders in storage.

curl --location 'MANAGER_ENDPOINT/folders/delete-from-trash-bin' \
--header 'Content-Type: application/json' \
--data '{
    "vfolder_id": "VFOLDER_ID"
}'

POST /folders/<NAME>/purge change to POST /folders/purge and get vfolder_id parameter

curl --location 'MANAGER_ENDPOINT/folders/purge' \
--header 'Content-Type: application/json' \
--data '{
    "vfolder_id": "VFOLDER_ID"
}'

Checklist: (if applicable)

Milestone metadata specifying the target backport version
Mention to the original issue
API server-client counterparts (e.g., manager API -> client SDK)
Test case(s) to:
- Demonstrate the difference of before/after
- Demonstrate the flow of abstract/conceptual models with a concrete implementation
Documentation
- Contents in the docs directory
- docstrings in public interfaces and type annotations

achimnol · 2024-02-22T01:52:17Z

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

+            print_error(e)
+            sys.exit(ExitCode.FAILURE)
+
+
 @vfolder.command()
 @click.argument("name", type=str)
 def recover(name):


Let's rename this to restore and keep the alias for backward compatibility.

I added restore command right below(line no 236) and left this for backward compatibility.

achimnol · 2024-02-22T01:54:27Z

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

+    pydantic_params_api_handler,
+    pydantic_response_api_handler,


I'd like to rename these to:

pydantic_api_params_handler

pydantic_api_reponse_handler

for consistency with composite nouns in a follow-up PR.

Let's postpone this refactoring to a follow-up PR.

achimnol · 2024-02-22T02:07:36Z

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

+    await ensure_vfolder_status(
+        request, VFolderAccessStatus.HARD_DELETABLE, folder_id_or_name=folder_id
+    )
+    async with root_ctx.db.begin() as conn:


This could be also a readonly connection or transaction.

achimnol · 2024-02-22T02:11:33Z

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

@@ -3296,14 +3446,13 @@ def create_app(default_cors_options):
    vfolder_resource = cors.add(app.router.add_resource(r"/{name}"))
    cors.add(vfolder_resource.add_route("GET", get_info))
    cors.add(vfolder_resource.add_route("DELETE", delete_by_name))
+    cors.add(add_route("GET", r"/{name}/id", get_vfolder_id))


I think it would be better to move the name parameter to a query param.

Since /folders/{name} may conflict with /folders/{invitations,purge,restore-from-trash-bin,delete-from-trash-bin}, I'd suggest using a new app prefix: /vfolders/ for new UUID-based API routes.

Let's postpone this refactoring to a follow-up PR.

- It is a follow-up to #1892 and #1936. - Adds `index=True` to the `kernels.role` column to avoid mismatch of DB and migrations.

refactor: re-define vfolder delete status

d0df459

fregataa added this to the 24.03 milestone Feb 6, 2024

fregataa requested a review from achimnol February 6, 2024 11:16

fregataa self-assigned this Feb 6, 2024

github-actions bot added comp:storage-proxy Related to Storage proxy component type:feature Add new features comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated size:L 100~500 LoC labels Feb 6, 2024

fregataa mentioned this pull request Feb 6, 2024

feat: improve reliability & usability of VFolder removal API #1884

Closed

5 tasks

fregataa added 6 commits February 6, 2024 20:18

add news fragment

6aca728

update news fragment

7f77f10

update status_chaged field

c7b9a85

remove keypair_resource_policy max_vfolder_count check in restore

f75f57b

apply status change in ensure API

cd3b660

rename recover to restore

8f30f80

github-actions bot added the comp:client Related to Client component label Feb 7, 2024

fregataa added 2 commits February 7, 2024 13:16

revert cli recover cmd and add restore cmd

eedc0a3

revert recover client API

e1e02a2

agatha197 mentioned this pull request Feb 7, 2024

feat: support vfolder trash bin lablup/backend.ai-webui#2204

Merged

14 tasks

fregataa added 2 commits February 7, 2024 14:16

refactor migration code

5f8d777

get vfolder id as API params

eb905ca

github-actions bot added size:XL 500~ LoC and removed size:L 100~500 LoC labels Feb 7, 2024

split dead vfolder statuses into soft deleted and hard deleted

b6d4856

fregataa marked this pull request as ready for review February 7, 2024 05:48

Merge branch 'main' into refactor/vfolder-delete-status

e2cc68a

github-actions bot added the urgency:4 As soon as feasible, implementation is essential. label Feb 11, 2024

fregataa added 2 commits February 14, 2024 11:22

Merge branch 'main' into refactor/vfolder-delete-status

79d49c5

rename some VFolderAccessStatus enum

a7f02b7

fregataa requested a review from agatha197 February 14, 2024 05:43

fregataa added 7 commits February 15, 2024 00:30

update delete_by_id API

78c3bbc

Merge branch 'main' into refactor/vfolder-delete-status

306f67d

do not check duplicate name with hard-deleted vfolder

8c44a35

update vfolder access status

f070fee

Merge branch 'main' into refactor/vfolder-delete-status

a4f3501

revert return SuccessResponseModel response from DELETE API

7c023f9

update CLI

f34f7ec

fregataa added the comp:cli Related to CLI component label Feb 15, 2024

fregataa requested a review from kyujin-cho February 16, 2024 02:09

fregataa added 2 commits February 16, 2024 18:36

Merge branch 'main' into refactor/vfolder-delete-status

3088695

Merge branch 'main' into refactor/vfolder-delete-status

84d0491

agatha197 approved these changes Feb 19, 2024

View reviewed changes

achimnol requested changes Feb 22, 2024

View reviewed changes

fregataa added 3 commits February 22, 2024 14:14

Merge branch 'main' into refactor/vfolder-delete-status

91566f0

readonly connection

022d0fa

update get_vfolder_id params

5147a29

achimnol approved these changes Feb 22, 2024

View reviewed changes

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

achimnol mentioned this pull request Feb 22, 2024

Advanced vfolder deletion features (aka trash bin) #767

Open

Merged via the queue into main with commit f5a8bbf Feb 22, 2024
26 checks passed

achimnol deleted the refactor/vfolder-delete-status branch February 22, 2024 06:54

achimnol mentioned this pull request Feb 28, 2024

fix: Rewrite the vfolder status migration in #1892 #1936

Merged

2 tasks

github-merge-queue bot pushed a commit that referenced this pull request Feb 28, 2024

fix: Rewrite the vfolder status migration in #1892 (#1936)

1fec5cf

achimnol added a commit that referenced this pull request Feb 28, 2024

fix: Add missing indexes in the vfolders table

982996b

- It is a follow-up to #1892 and #1936. - Adds `index=True` to the `kernels.role` column to avoid mismatch of DB and migrations.

achimnol mentioned this pull request Feb 28, 2024

fix: Add missing indexes in the vfolders table #1937

Merged

2 tasks

achimnol mentioned this pull request Apr 26, 2024

Clean up vfolder statuses #1915

Open

achimnol mentioned this pull request Jul 8, 2024

Enhance overall VFolder removal experience #2376

Open

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: re-define vfolder delete status #1892

refactor: re-define vfolder delete status #1892

fregataa commented Feb 6, 2024 •

edited

Loading

achimnol Feb 22, 2024

fregataa Feb 22, 2024

achimnol Feb 22, 2024

achimnol Feb 22, 2024

achimnol Feb 22, 2024

achimnol Feb 22, 2024

achimnol Feb 22, 2024

refactor: re-define vfolder delete status #1892

refactor: re-define vfolder delete status #1892

Conversation

fregataa commented Feb 6, 2024 • edited Loading

!! MUST

Updates

Updated/Added API

achimnol Feb 22, 2024

Choose a reason for hiding this comment

fregataa Feb 22, 2024

Choose a reason for hiding this comment

achimnol Feb 22, 2024

Choose a reason for hiding this comment

achimnol Feb 22, 2024

Choose a reason for hiding this comment

achimnol Feb 22, 2024

Choose a reason for hiding this comment

achimnol Feb 22, 2024

Choose a reason for hiding this comment

achimnol Feb 22, 2024

Choose a reason for hiding this comment

fregataa commented Feb 6, 2024 •

edited

Loading