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

♻️ Refactors webserver module director_v2 and breaks dependency cycles #2567

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Oct 4, 2021

What do these changes do?

This PR refactors web-server service to prepare for new features that will be introduce in a separate PR #2556

  • ♻️ Splits director-v2 app module into submodules for
    • _settings: Defines BaseSettings-based settings with options to configure this module
    • _handlers: defines handlers registered in the aiohttp's app.router
    • _core: includes core functionality
    • _api: exposes functions to other app modules
  • ♻️ Splits project app module's handler into smaller submodules
  • 🐛 Replaces built-in TimeoutError by asyncio.TimeoutError. Those are differemt things.
  • ♻️ Cleanup simcore_service_webserver.utils
  • ♻️ Removed dependency cycles.
$ cat $HOME/.pydeps
[pydeps]
max_bacon = 3
no_show = True
verbose = 1
pylib = False
exclude =
    os
    re
    sys
    collections
    __future__
    aiohttp
    __main__
    celery
    attr
    yaml
$ pip install pydeps   
$ pydeps src/simcore_service_webserver --show-cycles

image

Related issue/s

How to test

Checklist

@codecov
Copy link

codecov bot commented Oct 4, 2021

Codecov Report

Merging #2567 (df0bef2) into master (b0342a5) will increase coverage by 0.1%.
The diff coverage is 84.3%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #2567     +/-   ##
========================================
+ Coverage    76.8%   77.0%   +0.1%     
========================================
  Files         623     618      -5     
  Lines       24050   24073     +23     
  Branches     2357    2347     -10     
========================================
+ Hits        18483   18537     +54     
+ Misses       4938    4907     -31     
  Partials      629     629             
Flag Coverage Δ
integrationtests 65.6% <71.6%> (+0.1%) ⬆️
unittests 71.4% <82.6%> (+<0.1%) ⬆️

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

Impacted Files Coverage Δ
...b/server/src/simcore_service_webserver/__init__.py 100.0% <ø> (ø)
...rc/simcore_service_webserver/application_config.py 93.0% <ø> (ø)
.../server/src/simcore_service_webserver/constants.py 100.0% <ø> (ø)
...s/web/server/src/simcore_service_webserver/meta.py 72.7% <ø> (+8.4%) ⬆️
...erver/src/simcore_service_webserver/storage_api.py 50.0% <50.0%> (+0.9%) ⬆️
...ce_webserver/resource_manager/garbage_collector.py 74.7% <60.0%> (+0.8%) ⬆️
.../src/simcore_service_webserver/director_v2_core.py 66.9% <66.9%> (ø)
...re_service_webserver/projects/projects_handlers.py 83.4% <87.5%> (+0.7%) ⬆️
...simcore_service_webserver/projects/projects_api.py 83.3% <90.9%> (ø)
...rvice_webserver/projects/projects_tags_handlers.py 92.5% <92.5%> (ø)
... and 36 more

@pcrespov pcrespov changed the title Is2392/refactor directorv2 and dep cycles ♻️ Refactors directorv2 and breaks dependency cycles Oct 4, 2021
@pcrespov pcrespov self-assigned this Oct 4, 2021
@pcrespov pcrespov changed the title ♻️ Refactors directorv2 and breaks dependency cycles ♻️ Refactors webserver module director_v2 and breaks dependency cycles Oct 4, 2021
@pcrespov pcrespov added this to the Capra delle nevi milestone Oct 4, 2021
@pcrespov pcrespov added a:webserver issue related to the webserver service t:maintenance Some planned maintenance work labels Oct 4, 2021
@pcrespov pcrespov requested review from GitHK, sanderegg, colinRawlings and mguidon and removed request for GitHK October 4, 2021 08:25
@pcrespov pcrespov marked this pull request as ready for review October 4, 2021 08:29
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.

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'm a little bit scared of my next rebase ;)

@pcrespov pcrespov force-pushed the is2392/refactor-directorv2-and-dep-cycles branch from f66c4fd to 4f0b932 Compare October 4, 2021 09:50
@pcrespov pcrespov merged commit 1e0b895 into ITISFoundation:master Oct 4, 2021
@pcrespov pcrespov deleted the is2392/refactor-directorv2-and-dep-cycles branch October 4, 2021 17:18
@pcrespov pcrespov restored the is2392/refactor-directorv2-and-dep-cycles branch October 4, 2021 17:33
@pcrespov pcrespov deleted the is2392/refactor-directorv2-and-dep-cycles branch October 4, 2021 17:45
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 t:maintenance Some planned maintenance work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants