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

Is2911/fix delete project #2967

Merged
merged 36 commits into from
Apr 9, 2022

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Apr 7, 2022

What do these changes do?

New project deletion takes the following steps:

  1. marks project row as invisible (next PR will mark it as trashed)
  2. stops dynamic services
  3. stops computational services
  4. calls to delete data (next PR will mark it as trashed)
  5. deletes project (row database).

The key difference is that now the project is deleted last which keeps all the access-rights information necessary to perform the deletion in storage.

NEXT: A follow up to this case will be a trash for projects ITISFoundation/osparc-issues#468. There we will also extend delete workflows and integration tests.

Misc

  • ♻️ cleanup test_garbage_collector as well as some logs in gc plugin.
  • ♻️ split projects_handler since >500 lines
  • add storage info on show endpoint makefile recipe
  • fixed deprecation warning:
 DeprecationWarning: The 'asyncio_mode' is 'legacy', switching to 'auto' for the sake of pytest-aiohttp backward compatibility. Please explicitly use 'asyncio_mode=strict' or 'asyncio_mode=auto' in pytest configuration file.
    config.issue_config_time_warning(LEGACY_MODE, stacklevel=2)

Related issue/s

How to test

Manual testing

NOTE: will provide an internal entrypoint for testing on Friday

Automatic CI

pytest  tests/**/test_*delete*.py

will run

<Module tests/unit/with_dbs/02/test_projects_delete.py>
  <Function test_delete_project[UserRole.ANONYMOUS-expected0]>
  <Function test_delete_project[UserRole.GUEST-expected1]>
  <Function test_delete_project[UserRole.USER-expected2]>
  <Function test_delete_project[UserRole.TESTER-expected3]>
  <Function test_delete_multiple_opened_project_forbidden[UserRole.ANONYMOUS-HTTPUnauthorized-HTTPUnauthorized]>
  <Function test_delete_multiple_opened_project_forbidden[UserRole.GUEST-HTTPOk-HTTPForbidden]>
  <Function test_delete_multiple_opened_project_forbidden[UserRole.USER-HTTPOk-HTTPForbidden]>
  <Function test_delete_multiple_opened_project_forbidden[UserRole.TESTER-HTTPOk-HTTPForbidden]>

@pcrespov pcrespov self-assigned this Apr 7, 2022
@codecov
Copy link

codecov bot commented Apr 7, 2022

Codecov Report

Merging #2967 (cf01f53) into master (68a981d) will increase coverage by 0.0%.
The diff coverage is 86.1%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2967   +/-   ##
======================================
  Coverage    79.6%   79.6%           
======================================
  Files         681     683    +2     
  Lines       28417   28504   +87     
  Branches     3661    3671   +10     
======================================
+ Hits        22622   22692   +70     
- Misses       5027    5043   +16     
- Partials      768     769    +1     
Flag Coverage Δ
integrationtests 65.7% <71.1%> (+<0.1%) ⬆️
unittests 75.3% <84.5%> (+<0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...es/storage/src/simcore_service_storage/handlers.py 71.3% <50.0%> (ø)
...vice_webserver/exporter/formatters/formatter_v1.py 78.8% <66.6%> (+0.1%) ⬆️
...rvice_webserver/projects/projects_handlers_crud.py 82.8% <82.8%> (ø)
.../src/simcore_service_webserver/projects/_delete.py 85.1% <85.1%> (ø)
...imcore_service_webserver/garbage_collector_core.py 68.2% <91.3%> (+1.6%) ⬆️
...imcore_service_webserver/garbage_collector_task.py 100.0% <100.0%> (+5.1%) ⬆️
...mcore_service_webserver/garbage_collector_utils.py 79.4% <100.0%> (+1.3%) ⬆️
...es/web/server/src/simcore_service_webserver/log.py 66.6% <100.0%> (-2.6%) ⬇️
...imcore_service_webserver/meta_modeling_projects.py 82.0% <100.0%> (ø)
...r/src/simcore_service_webserver/projects/plugin.py 86.6% <100.0%> (ø)
... and 15 more

@pcrespov pcrespov force-pushed the is2911/fix-delete-project branch from 8aa12aa to 060cac4 Compare April 7, 2022 12:10
@pcrespov pcrespov force-pushed the is2911/fix-delete-project branch from b596b4d to 3db78ae Compare April 7, 2022 14:48
@pcrespov pcrespov marked this pull request as ready for review April 7, 2022 14:48
@pcrespov pcrespov requested review from sanderegg and GitHK as code owners April 7, 2022 14:48
@pcrespov pcrespov changed the title WIP: Is2911/fix delete project Is2911/fix delete project Apr 7, 2022
@pcrespov pcrespov marked this pull request as draft April 7, 2022 14:49
@pcrespov pcrespov force-pushed the is2911/fix-delete-project branch from b30e67b to b739a3d Compare April 7, 2022 17:16
@pcrespov pcrespov marked this pull request as ready for review April 7, 2022 17:16
@pcrespov pcrespov added a:webserver issue related to the webserver service changelog:🐛bugfix labels Apr 7, 2022
Copy link
Member

@mrnicegyu11 mrnicegyu11 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this contribution! Nice! :--)

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.

👍 Thanks for this.

Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

Saw it working 👌

Copy link
Contributor

@KZzizzle KZzizzle left a comment

Choose a reason for hiding this comment

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

thanks for the putting this together - definitely much needed!

@pcrespov pcrespov force-pushed the is2911/fix-delete-project branch from 57bf7a6 to 7a7863e Compare April 8, 2022 08:36
@pcrespov pcrespov requested a review from mguidon as a code owner April 8, 2022 08:36
@pcrespov pcrespov force-pushed the is2911/fix-delete-project branch from d9482b6 to 0b2a4f9 Compare April 8, 2022 16:26
@pcrespov pcrespov force-pushed the is2911/fix-delete-project branch from 0b2a4f9 to 3db6293 Compare April 9, 2022 11:09
@pcrespov pcrespov force-pushed the is2911/fix-delete-project branch from c01eb9f to 2806481 Compare April 9, 2022 15:50
@pcrespov pcrespov merged commit cd7d039 into ITISFoundation:master Apr 9, 2022
@pcrespov pcrespov deleted the is2911/fix-delete-project branch April 9, 2022 23:32
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
6 participants