Skip to content

BugFix: fixes awaitables and broad-exceptions in garbage-collector #2104

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

Merged
merged 9 commits into from
Feb 10, 2021

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Jan 25, 2021

What do these changes do?

Another group of fixes from PR #2079 associated with issues in the garbage collector.

  • broad exceptions catches were suppressing CancelledError which avoided the gc. Since the gc is a periodic background task it could never be cancelled
  • some extra fixes in garbage_collector.py
  • some coroutines were not awaited

How to test

make devenv
cd packages/postgres-database
make install-dev
make tests
cd ...
cd services/web/server
make install-dev
make test-dev-unit

cover this bugfix

@pcrespov pcrespov added the bug buggy, it does not work as expected label Jan 25, 2021
@pcrespov pcrespov self-assigned this Jan 25, 2021
@codecov
Copy link

codecov bot commented Jan 25, 2021

Codecov Report

Merging #2104 (86771fc) into master (5b03724) will increase coverage by 0.0%.
The diff coverage is 86.6%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2104   +/-   ##
======================================
  Coverage    73.4%   73.4%           
======================================
  Files         456     456           
  Lines       17195   17197    +2     
  Branches     1690    1689    -1     
======================================
+ Hits        12633   12637    +4     
+ Misses       4112    4111    -1     
+ Partials      450     449    -1     
Flag Coverage Δ
integrationtests 65.5% <75.5%> (+<0.1%) ⬆️
unittests 67.0% <73.3%> (-0.1%) ⬇️

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

Impacted Files Coverage Δ
...ce_webserver/resource_manager/garbage_collector.py 75.7% <82.3%> (-0.4%) ⬇️
.../sidecar/src/simcore_service_sidecar/log_parser.py 82.0% <100.0%> (ø)
...server/src/simcore_service_webserver/groups_api.py 91.6% <100.0%> (+<0.1%) ⬆️
...mcore_service_webserver/resource_manager/config.py 100.0% <100.0%> (ø)
.../server/src/simcore_service_webserver/users_api.py 93.9% <100.0%> (+<0.1%) ⬆️
...webserver/computation_comp_tasks_listening_task.py 91.0% <0.0%> (+3.3%) ⬆️

@pcrespov pcrespov added this to the The White Rabbit milestone Feb 9, 2021
@pcrespov pcrespov changed the title WIP bugfix: Fixes/awaitables Fixes/awaitables and broad-exceptions in garbage-collector Feb 10, 2021
@pcrespov pcrespov changed the title Fixes/awaitables and broad-exceptions in garbage-collector BugFix: fixes awaitables and broad-exceptions in garbage-collector Feb 10, 2021
@pcrespov pcrespov marked this pull request as ready for review February 10, 2021 13:32
@pcrespov pcrespov requested review from GitHK, sanderegg, odeimaiz, ignapas and mguidon and removed request for sanderegg February 10, 2021 13:32
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.

Very nice, I see you reduced the scope of error catching in the GC to only database errors.

I have only 2 rename requests

@pcrespov pcrespov merged commit 3bce414 into ITISFoundation:master Feb 10, 2021
@pcrespov pcrespov deleted the fixes/awaitables branch February 10, 2021 15:16
@sanderegg sanderegg mentioned this pull request Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug buggy, it does not work as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants