Skip to content

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

  • part of ITISFoundation/private-issues#6

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 webserver's codebase. Assigning the area is particularly useful for bugs label Feb 13, 2025
@codecov
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
Collaborator

@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
Collaborator

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
@sonarqubecloud
Copy link

@pcrespov pcrespov merged commit 8754b76 into ITISFoundation:master Feb 17, 2025
94 of 95 checks passed
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Mar 6, 2025
63 tasks
mrnicegyu11 pushed a commit to mrnicegyu11/osparc-simcore that referenced this pull request Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants