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

🔨 New settings-library for settings #2395

Merged
merged 29 commits into from
Jun 28, 2021

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Jun 21, 2021

What do these changes do?

NEW packages/settings-library package for settings : implements new settings designed drafted in sample-app-settings repo.

Highlights

  • settings_library.base defines new base class BaseCustomSettings:
    • implements create_from_envs constructor
    • sets BaseCustomSettings.Config
  • settings_library.cli_utils defines a factory to create settings commands for typer CLI
  • settings_library.celery, postgres, rabbit, redis, s3 reimplements models_library.settings.* using new settings design
    • all fields are UPPER_CASE
    • Uses @cached_property for read-only composed fields (e.g. urls). See tests for more details

In addition

  • ADDS new fixtures pytest_simcore.cli_runner
  • UPDATED storage service: replaces model_library.settings by settings_library
    • Adapts Settings
    • Simplifies CLI and improves tests

Related issue/s

This is one of the four PR issued to refactor settings. The other are:

  1. 🔨 Updates settings and removes config files: storage service #2369: refactors storage (merged)
  2. ♻️ Refactoring web-server settings and removing trafaret from the repo #2376: removes trafaret and refactors webserver
  3. Replaces model_library.settings by settings_library in the entire repo

How to test

make devenv
cd packages/settings-library
make install-dev
make tests

cd ...
cd services/storage
make install-dev
make tests

@pcrespov pcrespov self-assigned this Jun 21, 2021
@pcrespov pcrespov added this to the Marmoset milestone Jun 21, 2021
@codecov
Copy link

codecov bot commented Jun 21, 2021

Codecov Report

Merging #2395 (10846a8) into master (aad00a6) will decrease coverage by 0.7%.
The diff coverage is 85.7%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #2395     +/-   ##
========================================
- Coverage    74.1%   73.3%   -0.8%     
========================================
  Files         523     532      +9     
  Lines       20415   20576    +161     
  Branches     2017    2025      +8     
========================================
- Hits        15131   15091     -40     
- Misses       4766    4959    +193     
- Partials      518     526      +8     
Flag Coverage Δ
integrationtests 66.1% <ø> (-0.1%) ⬇️
unittests 66.7% <85.7%> (-1.1%) ⬇️

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

Impacted Files Coverage Δ
services/storage/src/simcore_service_storage/db.py 70.2% <ø> (ø)
...ervices/storage/src/simcore_service_storage/dsm.py 71.9% <ø> (ø)
...settings-library/src/settings_library/cli_utils.py 58.6% <58.6%> (ø)
...ervices/storage/src/simcore_service_storage/cli.py 58.8% <60.0%> (-2.0%) ⬇️
...es/settings-library/src/settings_library/celery.py 86.6% <86.6%> (ø)
...ings-library/src/settings_library/logging_utils.py 86.6% <86.6%> (ø)
.../settings-library/src/settings_library/postgres.py 92.8% <92.8%> (ø)
...ges/settings-library/src/settings_library/redis.py 93.3% <93.3%> (ø)
...ages/settings-library/src/settings_library/base.py 94.1% <94.1%> (ø)
...es/settings-library/src/settings_library/rabbit.py 94.1% <94.1%> (ø)
... and 28 more

@pcrespov pcrespov added the t:enhancement Improvement or request on an existing feature label Jun 21, 2021
@pcrespov pcrespov force-pushed the refactor/common-settings branch from 20947c1 to 98262b5 Compare June 27, 2021 18:59
@pcrespov pcrespov changed the title WIP: 🔨 New settings-library for all common settings 🔨 New settings-library for all common settings Jun 27, 2021
@pcrespov pcrespov changed the title 🔨 New settings-library for all common settings 🔨 New settings-library for settings classes Jun 27, 2021
@pcrespov pcrespov changed the title 🔨 New settings-library for settings classes 🔨 New settings-library for settings Jun 27, 2021
@pcrespov pcrespov marked this pull request as ready for review June 27, 2021 22:31
@pcrespov pcrespov force-pushed the refactor/common-settings branch from 6754842 to 358c700 Compare June 28, 2021 06:52
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.

Just a few minor comments.

@pcrespov pcrespov merged commit 663fd9b into ITISFoundation:master Jun 28, 2021
@pcrespov pcrespov deleted the refactor/common-settings branch June 28, 2021 16:10
@pcrespov pcrespov mentioned this pull request Jul 5, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:enhancement Improvement or request on an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants