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

✨ web-api: Empty bin of explicitly trashed projects #7226

Merged

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Feb 13, 2025

What do these changes do?

ReDoc

This PR introduces the initial implementation of delete_trashed_project, focusing on deleting explicitly trashed projects that are not shared. Future PRs will extend this policy to cover additional cases.

Highlights

  • web-api: v0.58.0: implements Empty Trash (limited to projects explicitly trashed)
    • Renames the empty_trash endpoint to POST /trash:empty.
    • Implemented to have immediate response as the deletion mechanism is started ( fire&forget empty_trash_safe )
    • Trashes projects marked explicitly as trashed. Errors are ignored but logged.
    • This is connected to "Empty Trash" button in the front-end (which is still not enabled until the deletion policy is extended to all trashable items)
  • pagination_tools: Adds helper functions for iterating over paginated listings.
  • delete_project_by_user:
    • Question to Reviewers: Should a locking mechanism be implemented during deletion? Given that trashed projects are not listed as active, they should, in principle, not be subject to any operations. So i wonder if it is worth implementing that protection mechanism. Feedback is welcome.

Next steps

  • deletion of folders and all implicit projects and folders
  • deletion of shared projects
  • deletion of workspace
  • background task to delete after retention time expires

Related issue/s

How to test

  • Driving tests
cd services/web/server
make install-dev
pytest -vv tests/unit/with_dbs/04/test*trash*.py
pytest -vv tests/unit/with_dbs/04/test_trash.py::test_trash_project_explitictly_and_empty_trash_bin
  • Manual test
    • created a study with data (1GB)
    • duplicate study
    • check new projects in database and minio
    • trash duplications
    • list trash
    • from the API POST /trash:empty
    • checked database: projects are deleted
    • checked minio: data is deleted

Dev-ops

None

@pcrespov pcrespov self-assigned this Feb 13, 2025
@pcrespov pcrespov added the a:webserver issue related to the webserver service label Feb 13, 2025
Copy link

codecov bot commented Feb 13, 2025

Codecov Report

Attention: Patch coverage is 94.68085% with 5 lines in your changes missing coverage. Please review.

Project coverage is 88.24%. Comparing base (bcaa959) to head (9b868bb).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7226      +/-   ##
==========================================
+ Coverage   86.99%   88.24%   +1.25%     
==========================================
  Files        1667     1166     -501     
  Lines       64721    49614   -15107     
  Branches     1096      202     -894     
==========================================
- Hits        56302    43783   -12519     
+ Misses       8106     5762    -2344     
+ Partials      313       69     -244     
Flag Coverage Δ
integrationtests 65.25% <44.64%> (-0.03%) ⬇️
unittests 86.98% <93.61%> (+0.99%) ⬆️
Components Coverage Δ
api 76.84% <ø> (ø)
pkg_aws_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 85.46% <ø> (ø)
agent 96.46% <ø> (ø)
api_server 90.56% <ø> (ø)
autoscaling 96.08% <ø> (ø)
catalog 91.71% <ø> (ø)
clusters_keeper 99.24% <ø> (ø)
dask_sidecar 91.25% <ø> (ø)
datcore_adapter 93.19% <ø> (ø)
director 76.59% <ø> (ø)
director_v2 91.30% <ø> (ø)
dynamic_scheduler 97.33% <ø> (ø)
dynamic_sidecar 89.77% <ø> (ø)
efs_guardian 90.25% <ø> (ø)
invitations 93.28% <ø> (ø)
osparc_gateway_server ∅ <ø> (∅)
payments 92.66% <ø> (ø)
resource_usage_tracker 89.08% <ø> (+0.10%) ⬆️
storage 86.67% <ø> (+0.05%) ⬆️
webclient ∅ <ø> (∅)
webserver 84.81% <92.85%> (+0.05%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bcaa959...9b868bb. Read the comment docs.

@pcrespov pcrespov force-pushed the is468/empty-trash-from-explicit-projects branch from 5594adf to 7483893 Compare February 13, 2025 19:17
@pcrespov pcrespov added this to the Singularity milestone Feb 13, 2025
@pcrespov pcrespov force-pushed the is468/empty-trash-from-explicit-projects branch from 7483893 to 1a6e18b Compare February 14, 2025 14:11
@pcrespov pcrespov changed the title WIP: ✨ Is468/empty trash from explicit projects ✨ web-api: Empty bin of explicitly trashed projects Feb 17, 2025
@pcrespov pcrespov force-pushed the is468/empty-trash-from-explicit-projects branch from 814c8f6 to e9e3062 Compare February 17, 2025 08:15
@pcrespov pcrespov marked this pull request as ready for review February 17, 2025 08:15
@pcrespov pcrespov enabled auto-merge (squash) February 17, 2025 10:09
@giancarloromeo
Copy link
Contributor

Q: We are implementing "delayed" operations using Distributed Task Queues... what happens if the project is shared with somebody that queued a "cloning" operation task before the deletion?

Copy link
Contributor

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@matusdrobuliak66
Copy link
Contributor

Should a locking mechanism be implemented during deletion? -> Maybe it’s not necessary right now, but if additional functionality such as data deletion is added in the future, it might be useful.

@pcrespov
Copy link
Member Author

Q: We are implementing "delayed" operations using Distributed Task Queues... what happens if the project is shared with somebody that queued a "cloning" operation task before the deletion?

@giancarloromeo good point. Note, nonetheless, that items are marked as trashed, i.e. in principle they are not clonable. There is a locking mechanism when trashing ... i.e. if it is cloning, it cannot be trashed.

@pcrespov pcrespov force-pushed the is468/empty-trash-from-explicit-projects branch from 3859ba3 to 9b868bb Compare February 17, 2025 10:54
@pcrespov pcrespov merged commit 8754b76 into ITISFoundation:master Feb 17, 2025
94 of 95 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:webserver issue related to the webserver service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants