♻️ gc as a service (preparation)

[pcrespov]

What do these changes do?

In preparation to have a garbage-collector (gc) as a stand-alone service and separate from the the web-server ( coming in PR #2819) , the web-server needed some refactoring. Most of the work focus on two ideas:

• activating enough of the new settings to enable/disable plugins while still keeping config legacy in place (we are being very cautious with this because it affects operations)
• reducing coupling between resource_manager plugin (where gc functionality is) and the rest of the web-server modules

That lead to two main changes:

• app_module_setup decorator uses ApplicationSettings.WEBSERVER_* values to disable addons
• gc and redis-client moved from resource_manager plugin to stand-alone plugins called garbage_collector and redis resp.

Plugins Design Rationale (reminder)

The web-server consists of multiple plugins (also referred as app modules) that can be enable/disabled via env vars to compose an app with different functionality.

Each plugin consists of one or more python modules with the same name prefix or under the same folder. It has settings (added as a section in the application_settings.ApplicationSettings) and has a setup function that is called during application.create_application.

For instance, a hypothetical myplugin plugin might have this skeleton

...
myplugin_core.py       # or myplugin/_core.py
myplugin_settings.py   # or myplugin/settings.py
myplugin.py            # or myplugin/module_setup.py
...

In myplugin_settings.py defines the plugin's settings

# myplugin_settings.py

class MyPluginSettings(BaseCustomSettings):
    MY_PLUGIN_VALUE: int

# this will be available in app[APP_SETTINGS_KEY]

In myplugin.pya setup function that is registered using the app_module_setup decorator.

# myplugin.py or /myplugin/module_setup.py

@app_module_setup ("simcore_service_webserver.activity", category=ModuleCategory.ADDON)
def setup_myplugin(app: web.Application):
    settings = app[APP_SETTINGS_KEY].WEBSERVER_MYPLUGIN
    # init some client api (e.g. pg engine or httpx client)
    # add routes to webserver (e.g. gateway to catalog, ...)
    # add ``on_startup``, ``on_cleanup``, ``cleanup_ctx`` 

To disable the plugin, we just need to set WEBSERVER_MYPLUGIN=null the setup will be skip. That means that all the files myplugin_* could eventually only be imported to be used as a library.

With this PR, we will be able to enable all plugins to compose a web-server with only the functionality of the garbage-collector. Then we can create a dedicated service in the stack (e.g. webserver-garbage-collector) responsible for that task and remove that responsibility from the current webserver.

NOTE: @mrnicegyu11 @Surfict @colinRawlings @KZzizzle rationale above might be of interest for you.

Related issue/s

• preparation for PR WIP: ⚗️ Is2757/gc as a service #2819

How to test

web-server tests cover all changes

Checklist

• Openapi changes? make openapi-specs, git commit ... and then make version-*)
• Database migration script? cd packages/postgres-database, make setup-commit, sc-pg review -m "my changes"
• Unit tests for the changes exist
• Runs in the swarm
• Documentation reflects the changes
• New module? Add your github username to .github/CODEOWNERS

[codecov]

Codecov Report

Merging #2828 (7ca4ec6) into master (137234a) will decrease coverage by 0.1%.
The diff coverage is 77.6%.

@@           Coverage Diff            @@
##           master   #2828     +/-   ##
========================================
- Coverage    78.9%   78.7%   -0.2%     
========================================
  Files         671     673      +2     
  Lines       27290   27336     +46     
  Branches     3174    3182      +8     
========================================
- Hits        21550   21534     -16     
- Misses       4978    5041     +63     
+ Partials      762     761      -1     

Flag	Coverage Δ
integrationtests	65.7% <73.2%> (-0.3%)	⬇️
unittests	74.3% <77.1%> (-0.2%)	⬇️

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

Impacted Files	Coverage Δ
...c/simcore_service_webserver/application__schema.py	93.5% <ø> (ø)
...eb/server/src/simcore_service_webserver/session.py	70.8% <ø> (-2.3%)	⬇️
...simcore_service_webserver/socketio/module_setup.py	100.0% <ø> (ø)
...webserver/studies_dispatcher/handlers_redirects.py	85.5% <ø> (ø)
...es/web/server/src/simcore_service_webserver/cli.py	51.8% <13.0%> (-4.7%)	⬇️
...ibrary/src/servicelib/aiohttp/application_setup.py	78.1% <67.3%> (-4.8%)	⬇️
.../server/src/simcore_service_webserver/users_api.py	92.0% <70.0%> (-2.3%)	⬇️
...imcore_service_webserver/garbage_collector_core.py	70.1% <77.7%> (ø)
.../simcore_service_webserver/application_settings.py	97.5% <85.7%> (-0.8%)	⬇️
...imcore_service_webserver/garbage_collector_task.py	94.7% <94.7%> (ø)
... and 26 more

[GitHK]

Looking nice.

One thing. Why refer to the same item with the words plugin and add-on? Can't we just use one of them to avoid confusion?

[sanderegg]

Great! please just have a look at my comments and especially about the resource manager plugin being an ADDON vs SYSTEM where I am not sure that is safe to do

[sanderegg]

why the else here?

[pcrespov]

it is part of try ... except ... else. From https://docs.python.org/3/tutorial/errors.html#handling-exceptions

The try … except statement has an optional else clause, which, when present, must follow all except clauses. It is useful for code that must be executed if the try clause does not raise an exception

[sanderegg]

I am not sure I get the meaning of that comment. you raise an exception if a dependency is missing right? is that not enforcing?

[pcrespov]

Right, my point here is that perhaps there is no need to enforce that those were already initialized but instead e.g. the decorator's wrapper could could them itself. It is just a note to think about it ... not relevant for this PR.
The dependencies is a design feature I got from s4l but my experience tells me that it does not really get updated.

[sanderegg]

just a thought but I wonder if we should not update that one once... current version is 14.x

[pcrespov]

@sanderegg please add a maintenance case for it and we can follow up

[sanderegg]

this one I do agree. this can be an ADDON since this is something that does not prevent the webserver from functioning

[pcrespov]

this is not the case for the webserver-GC . AFAIK it is not needed there, right?

[KZzizzle]

thanks for the notice and for the detailed write-up!

[GitHK]

👍

[mrnicegyu11]

thx