🐛♻️ Deleting a project now removes its files

[GitHK]

What do these changes do?

• Fixes and issue where the user had no permissions to delete the project data (after the project was removed).
• Added integration test to catch if feature breaks in the future, based off the importer testing
• refactored integration/01 for the webserver

Related issue/s

• closes storage does not delete files from project #2754

How to test

Checklist

[codecov]

Codecov Report

Merging #2777 (90a5d32) into master (441542e) will decrease coverage by 1.0%.
The diff coverage is 52.6%.

@@           Coverage Diff            @@
##           master   #2777     +/-   ##
========================================
- Coverage    78.9%   77.8%   -1.1%     
========================================
  Files         670     670             
  Lines       27236   27250     +14     
  Branches     2678    2679      +1     
========================================
- Hits        21495   21219    -276     
- Misses       4981    5284    +303     
+ Partials      760     747     -13     

Flag	Coverage Δ
integrationtests	62.3% <ø> (-3.5%)	⬇️
unittests	74.4% <52.6%> (-0.1%)	⬇️

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

Impacted Files	Coverage Δ
...ervices/storage/src/simcore_service_storage/dsm.py	73.2% <28.5%> (-0.3%)	⬇️
...torage/src/simcore_service_storage/access_layer.py	72.7% <66.6%> (-1.1%)	⬇️
...tor_v2/modules/dynamic_sidecar/scheduler/events.py	38.8% <0.0%> (-55.9%)	⬇️
...ules/dynamic_sidecar/docker_service_specs/proxy.py	53.8% <0.0%> (-46.2%)	⬇️
...v2/modules/dynamic_sidecar/docker_compose_specs.py	54.6% <0.0%> (-45.4%)	⬇️
.../modules/dynamic_sidecar/scheduler/events_utils.py	55.5% <0.0%> (-44.5%)	⬇️
...s/dynamic_sidecar/docker_service_specs/settings.py	33.1% <0.0%> (-31.0%)	⬇️
..._director_v2/modules/dynamic_sidecar/client_api.py	54.2% <0.0%> (-27.9%)	⬇️
...tor_v2/modules/dynamic_sidecar/volumes_resolver.py	64.1% <0.0%> (-20.6%)	⬇️
...vice_director_v2/modules/dynamic_sidecar/errors.py	82.6% <0.0%> (-17.4%)	⬇️
... and 11 more

[sanderegg]

Very nice!
Please just add a tenacity retrial instead of a hard-coded sleep time in your test.

[pcrespov]

Good catch of the bug and nice refactoring of the tests.
I still have some questions and extra suggestions. I am happy to zoom to clarify but i think there are a couple of important points to deal with.

thx

[odeimaiz]

We will face a new problem when this PR goes in.
Right now, you can use/link files from a Study B in your Study A. When Study B, together with its data, gets deleted, Study A will no longer be able find the input data.

How are you currently doing this linking? Can't it be change it to copy the data?

@sanderegg @pcrespov were you aware?

Right now, the link is done as follows:
FilePickerA in StudyA contains the "path:StudyB/FilePickerB/file".

There are two solutions here:

1. Before deleting the data from StudyB, you check if that file is used somewhere else. If so
   • Do not delete the used content
   • Copy it to the study/folder where it used and update the link in the project
2. When a link to a different study is detected, the file gets copied

I would go for approach number 2

[GitHK]

We will face a new problem when this PR goes in.
Right now, you can use/link files from a Study B in your Study A. When Study B, together with its data, gets deleted, Study A will no longer be able find the input data.

How are you currently doing this linking? Can't it be change it to copy the data?
@sanderegg @pcrespov were you aware?

Right now, the link is done as follows: FilePickerA in StudyA contains the "path:StudyB/FilePickerB/file".

There are two solutions here:

1. Before deleting the data from StudyB, you check if that file is used somewhere else. If so

   • Do not delete the used content
   • Copy it to the study/folder where it used and update the link in the project

2. When a link to a different study is detected, the file gets copied

I would go for approach number 2

I would also go for number 2 and ask for a bit of frontend coordination. I would make it so that when the user selects such a file in the file picker, an API on the backend is called to do the copy. I think this might be the simplest solution.

[odeimaiz]

We will face a new problem when this PR goes in.
Right now, you can use/link files from a Study B in your Study A. When Study B, together with its data, gets deleted, Study A will no longer be able find the input data.

How are you currently doing this linking? Can't it be change it to copy the data?
@sanderegg @pcrespov were you aware?

Right now, the link is done as follows: FilePickerA in StudyA contains the "path:StudyB/FilePickerB/file".
There are two solutions here:

1. Before deleting the data from StudyB, you check if that file is used somewhere else. If so

   • Do not delete the used content
   • Copy it to the study/folder where it used and update the link in the project

2. When a link to a different study is detected, the file gets copied

I would go for approach number 2

I would also go for number 2 and ask for a bit of frontend coordination. I would make it so that when the user selects such a file in the file picker, an API on the backend is called to do the copy. I think this might be the simplest solution.

Such an API already exists:
https://github.com/ITISFoundation/osparc-simcore/blob/master/api/specs/webserver/openapi-storage.yaml#L143

What I don't know is if it still works. Ideally it should somewhere provide some copy progress info.

[pcrespov]

OK, I thought this was clear by but I see there are still doubts ... so let's talk next week and clarify still doubts.

In the meantime:

• ExporterSettings() works on its own because is a BaseCustomSettings so with the constructor it will capture the env settings
• Nonetheless, the new settings approach enforces all settings to be captured at the same time as fields of ApplicationSettings which is then saved in app[APP_SETTINGS_KEY].WEBSERVER_EXPORTER
• Now, if you did the test fixture right, both app[APP_SETTINGS_KEY].WEBSERVER_EXPORTER and ExporterSettings() should work

[pcrespov]

1. Before deleting the data from StudyB, you check if that file is used somewhere else. If so

   • Do not delete the used content
   • Copy it to the study/folder where it used and update the link in the project

2. When a link to a different study is detected, the file gets copied

I would go for approach number 2

I would also go for number 2 and ask for a bit of frontend coordination. I would make it so that when the user selects such a file in the file picker, an API on the backend is called to do the copy. I think this might be the simplest solution.

In 2., if I understand it correctly, besides copying the file to StudyA, it would require also changing the FilePickerA in StudyA such that now it contains the "path:StudyA/FilePickerA/file", right? So it implies also a change in StudyA's project.

• What if this project is open/active?

[GitHK]

1. Before deleting the data from StudyB, you check if that file is used somewhere else. If so

   • Do not delete the used content
   • Copy it to the study/folder where it used and update the link in the project

2. When a link to a different study is detected, the file gets copied

I would go for approach number 2

I would also go for number 2 and ask for a bit of frontend coordination. I would make it so that when the user selects such a file in the file picker, an API on the backend is called to do the copy. I think this might be the simplest solution.

In 2., if I understand it correctly, besides copying the file to StudyA, it would require also changing the FilePickerA in StudyA such that now it contains the "path:StudyA/FilePickerA/file", right? So it implies also a change in StudyA's project.

• What if this project is open/active?

In that case a lock should be shown in the UI and the user should not be able to pick that file.

[GitHK]

1. Before deleting the data from StudyB, you check if that file is used somewhere else. If so

   • Do not delete the used content
   • Copy it to the study/folder where it used and update the link in the project

2. When a link to a different study is detected, the file gets copied

I would go for approach number 2

I would also go for number 2 and ask for a bit of frontend coordination. I would make it so that when the user selects such a file in the file picker, an API on the backend is called to do the copy. I think this might be the simplest solution.

In 2., if I understand it correctly, besides copying the file to StudyA, it would require also changing the FilePickerA in StudyA such that now it contains the "path:StudyA/FilePickerA/file", right? So it implies also a change in StudyA's project.

• What if this project is open/active?

In that case a lock should be shown in the UI and the user should not be able to pick that file.

After discussing with @sanderegg we agreed on the following:

• when removing a project, we check that the files in the filepickers are not present in other projects
• Suppose a file is both part of Project A and Project B; also the file was originally part of Project. If Project A and getes removed, the file picker in project B will still join to a file counting Project A's project_id and node_id.

@pcrespov @odeimaiz

[GitHK]

@pcrespov I think this is no longer relevant with the work you already did. Should we just drop it?

[pcrespov]

@pcrespov I think this is no longer relevant with the work you already did. Should we just drop it?

right. Yes, i think that the fix in #2967 will solve thisone as well. In any case, i will carefully check again these changes and make take some to the other PR. Thank @GitHK

[GitHK]

@pcrespov I think this is no longer relevant with the work you already did. Should we just drop it?

right. Yes, i think that the fix in #2967 will solve thisone as well. In any case, i will carefully check again these changes and make take some to the other PR. Thank @GitHK

If that is the case make sure to add a - closes https://github.com/ITISFoundation/osparc-simcore/pull/2777 to your PR under the related issues so this gets closed when you close yours.

[sanderegg]

this gets replaced.

[GitHK]

replaced by #2967