Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Feb 18, 2025

What do these changes do?

This PR is a continuation of the work on the trash feature. Specifically:

  • delete expired trashed items as "admin-user" (i.e. w/o access-rights check):
    • implemented in trash._service.delete_expired_trash
    • expose api for background task (trash.trash_service)
  • background task runs periodically in the webserver-GC service to delete trashed items over the retention time
    • implemented as a task in the GC service (uses legacy style for bg tasks for now as the rest in the GC)
    • retention time given by TRASH_RETENTION_DAYS
    • periodicity of GC for this task give by GARBAGE_COLLECTOR_PRUNE_APIKEYS_INTERVAL_S (for now)
  • Misc
    • ♻️ refactoring feature example
    • 🔨 new test functions for Unset
    • 🔨 enhances test errors
    • 🐛♻️ Added the TRASH_RETENTION_DAYS missing env var to the webserver-GC option and sorted env-vars in the docker-compose

Next steps

Related issue/s

  • part of ITISFoundation/private-issues#6

How to test

cd services/web/service
make install-dev
pytest -vv -k test_trash_service__delete_expired_trash tests/unit/with_db 
pytest -vv tests/unit/with_db/**/test_*trash*.py

Dev-ops

  • changes in osparc-config not needed

@pcrespov pcrespov self-assigned this Feb 18, 2025
@codecov
Copy link

codecov bot commented Feb 18, 2025

Codecov Report

Attention: Patch coverage is 86.84211% with 30 lines in your changes missing coverage. Please review.

Project coverage is 87.77%. Comparing base (ba9624a) to head (2548ba8).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7246      +/-   ##
==========================================
+ Coverage   87.08%   87.77%   +0.68%     
==========================================
  Files        1679     1671       -8     
  Lines       64869    64056     -813     
  Branches     1115     1140      +25     
==========================================
- Hits        56491    56222     -269     
+ Misses       8060     7509     -551     
- Partials      318      325       +7     
Flag Coverage Δ
integrationtests 65.46% <54.22%> (+0.03%) ⬆️
unittests 86.07% <83.77%> (-0.05%) ⬇️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_library 94.17% <ø> (ø)
pkg_dask_task_models_library 97.09% <ø> (ø)
pkg_models_library 91.54% <100.00%> (ø)
pkg_notifications_library 84.57% <ø> (ø)
pkg_postgres_database 88.28% <ø> (ø)
pkg_service_integration 70.03% <ø> (ø)
pkg_service_library 72.61% <ø> (ø)
pkg_settings_library 90.61% <ø> (ø)
pkg_simcore_sdk 85.46% <ø> (+0.37%) ⬆️
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% <ø> (-0.10%) ⬇️
director_v2 91.29% <ø> (+0.01%) ⬆️
dynamic_scheduler 97.33% <ø> (ø)
dynamic_sidecar 89.74% <ø> (-0.04%) ⬇️
efs_guardian 90.25% <ø> (ø)
invitations 93.28% <ø> (ø)
osparc_gateway_server ∅ <ø> (∅)
payments 92.66% <ø> (ø)
resource_usage_tracker 89.13% <ø> (-0.06%) ⬇️
storage 86.73% <ø> (+0.05%) ⬆️
webclient ∅ <ø> (∅)
webserver 87.50% <87.11%> (+2.49%) ⬆️

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 ba9624a...2548ba8. Read the comment docs.

@pcrespov pcrespov force-pushed the is468/rm-expired-trash branch 2 times, most recently from 2125de1 to 861b2f2 Compare February 18, 2025 22:26
@pcrespov pcrespov added this to the Singularity milestone Feb 19, 2025
@pcrespov pcrespov added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Feb 19, 2025
@pcrespov pcrespov force-pushed the is468/rm-expired-trash branch 2 times, most recently from 6a2e54f to 5d9fc81 Compare February 19, 2025 16:27
@pcrespov pcrespov marked this pull request as ready for review February 20, 2025 09:27
@pcrespov pcrespov force-pushed the is468/rm-expired-trash branch from 58116c4 to 9da3764 Compare February 20, 2025 09:27
@pcrespov pcrespov changed the title WIP: ✨ Is468/rm expired trash Deletion of trashed items upon expiration of retention time Feb 20, 2025
@pcrespov pcrespov changed the title Deletion of trashed items upon expiration of retention time ✨ Deletion of trashed items upon expiration of retention time Feb 20, 2025
@pcrespov pcrespov changed the title ✨ Deletion of trashed items upon expiration of retention time ✨ Deletion of trashed projects/folders upon expiration of retention time Feb 20, 2025
@matusdrobuliak66
Copy link
Collaborator

Thanks!

Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

Q: where is this ran, in which service? The webserver or the GC?
How do you manage to make this only run in one copy?

@pcrespov
Copy link
Member Author

pcrespov commented Feb 20, 2025

Q: where is this ran, in which service? The webserver or the GC? How do you manage to make this only run in one copy?

This is what I tried to explain in the description :-) HEre some concrete answers

Q: where is this ran, in which service?

the background is in the webserver-GC (see tasks_trash)

How do you manage to make this only run in one copy?

There is only one replica of the webserver-GC. The tasks of this service suffer from this issue as well, so I plan to upgrade them all (but in a separate PR, otherwise it is messy)
For now, as I mentioned above, this PR uses the current background task mechanism already in the GC

@pcrespov pcrespov force-pushed the is468/rm-expired-trash branch from c6a81f8 to 1433f19 Compare February 20, 2025 15:46
@sonarqubecloud
Copy link

@pcrespov pcrespov enabled auto-merge (squash) February 20, 2025 17:29
@pcrespov pcrespov merged commit 7be787f into ITISFoundation:master Feb 20, 2025
87 of 95 checks passed
@pcrespov pcrespov deleted the is468/rm-expired-trash branch February 20, 2025 17:31
@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.

7 participants