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

✨Adds reserved disk space to dynamic-sidecar #5161

Merged
merged 21 commits into from
Dec 15, 2023

Conversation

GitHK
Copy link
Contributor

@GitHK GitHK commented Dec 12, 2023

What do these changes do?

When the dynamic-sidecar starts it now reserves a block of 10MiB from the disk to be freed in case of emergencies.

If the underlying hosts runs out of disk space, this comes in handy.

Related issue/s

How to test

Dev Checklist

DevOps Checklist

@GitHK GitHK self-assigned this Dec 12, 2023
@GitHK GitHK added a:director-v2 issue related with the director-v2 service a:dynamic-sidecar dynamic-sidecar service labels Dec 12, 2023
@GitHK GitHK added this to the Kobayashi Maru milestone Dec 12, 2023
Copy link

codecov bot commented Dec 12, 2023

Codecov Report

Merging #5161 (fcbfce6) into master (4c3ebdf) will decrease coverage by 19.1%.
The diff coverage is 72.8%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #5161      +/-   ##
=========================================
- Coverage    87.3%   68.3%   -19.1%     
=========================================
  Files        1280     531     -749     
  Lines       52567   26840   -25727     
  Branches     1141     198     -943     
=========================================
- Hits        45943   18357   -27586     
- Misses       6379    8433    +2054     
+ Partials      245      50     -195     
Flag Coverage Δ
integrationtests 64.9% <70.0%> (+<0.1%) ⬆️
unittests 88.0% <100.0%> (+2.7%) ⬆️

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

Files Coverage Δ
...r_v2/modules/dynamic_sidecar/api_client/_public.py 89.3% <100.0%> (-3.9%) ⬇️
...tor_v2/modules/dynamic_sidecar/api_client/_thin.py 96.8% <100.0%> (-3.3%) ⬇️
...ector_v2/modules/dynamic_sidecar/scheduler/_abc.py 100.0% <100.0%> (ø)
...s/dynamic_sidecar/scheduler/_core/_events_utils.py 91.8% <100.0%> (-1.0%) ⬇️
...rc/simcore_service_dynamic_sidecar/api/_routing.py 100.0% <100.0%> (ø)
...ar/src/simcore_service_dynamic_sidecar/api/disk.py 100.0% <100.0%> (ø)
...imcore_service_dynamic_sidecar/core/application.py 97.2% <100.0%> (+<0.1%) ⬆️
...ore_service_dynamic_sidecar/core/reserved_space.py 100.0% <100.0%> (ø)
...c/simcore_service_dynamic_sidecar/core/settings.py 96.9% <100.0%> (+<0.1%) ⬆️
...ervice_director_v2/api/routes/dynamic_scheduler.py 58.9% <66.6%> (-35.4%) ⬇️
... and 5 more

... and 1029 files with indirect coverage changes

@GitHK GitHK marked this pull request as ready for review December 13, 2023 08:31
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.

👍

Copy link

codeclimate bot commented Dec 13, 2023

Code Climate has analyzed commit 105f60c and detected 0 issues on this pull request.

View more on Code Climate.

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

Ok I like the idea. You reserve some space that is always available for the preferences. That is very nice.

Now I have a few suggestions/questions here:

  • can you maybe rename this space from "emergency" to "reserved" ?
  • would it not be possible to automatically free it just before saving the preferences instead of all that manual stuff that I guess you will need to do if this happens?
  • alternatively, why not have the preferences with the current values/defaults saved on start so that the space is already occupied and just needs to be rewritten?
  • I guess this would also include possible screenshots? then the space might not be sufficient.

@GitHK
Copy link
Contributor Author

GitHK commented Dec 14, 2023

Ok I like the idea. You reserve some space that is always available for the preferences. That is very nice.

Now I have a few suggestions/questions here:

  • can you maybe rename this space from "emergency" to "reserved" ?

  • would it not be possible to automatically free it just before saving the preferences instead of all that manual stuff that I guess you will need to do if this happens?

good point, did not think about it. will still leave in the functionality to remove it manually (since stuff can always go wrong)

  • alternatively, why not have the preferences with the current values/defaults saved on start so that the space is already occupied and just needs to be rewritten?
  • I guess this would also include possible screenshots? then the space might not be sufficient.

I did not understand these last two points

@GitHK GitHK requested a review from sanderegg December 14, 2023 08:23
@GitHK GitHK changed the title ✨Adds emergency disk space to dynamic-sidecar ✨Adds reserved disk space to dynamic-sidecar Dec 14, 2023
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

thank you!

@GitHK GitHK enabled auto-merge (squash) December 15, 2023 08:47
Copy link

sonarcloud bot commented Dec 15, 2023

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.1% Duplication on New Code

See analysis details on SonarCloud

@GitHK GitHK merged commit f88569e into ITISFoundation:master Dec 15, 2023
54 checks passed
@GitHK GitHK deleted the pr-osparc-add-emergency-disk-space branch December 15, 2023 10:32
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Jan 8, 2024
33 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:director-v2 issue related with the director-v2 service a:dynamic-sidecar dynamic-sidecar service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants