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

♻️ Is638/dynamic sidecar (round 5) #3165

Merged

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Jul 5, 2022

What do these changes do?

This PR is mostly addressing some helper functions and tooling. It is a bit noisy but the most relevant changes are:

pytest_simcore

  • ♻️ changes in env vars tools for testing. Fixes some inconsistencies in pytest_simcore.helpers.utils_envs

servicelib:

  • ✨ Osparc Error Codes (OEC) for user messages and trace error in log (see use case in test_errors.py and in update_failing_status)
  • 🔨 servicelib.fastapi.openapi.get_common_oas_options adds default options for OAS info. In particular a server entry for development that shows in swagger doc. This will simplify manual testing an API by just going to http://127.0.0.1:$(published_port)/dev/doc and get

director-v2:

  • ♻️ Simplifies some logs and uses OECs to message user
  • ♻️ settings: dy-sidecar is spawned in the same boot mode as the director-v2
    - in devel mode, dy-sidecar mounts volumes and exposes http and debugger ports as any other service.
  • ♻️ simplifies env-vars fixtures (NOTE: we can reduce a lot of test code by simply overriding some basic environs)
  • 🔨 updates OAS openapi.json

dynamic-services:

  • 🔨 updates OAS openapi.json
  • api: reserved id in entrypoints

Related issue/s

How to test

CI of packages above

@pcrespov pcrespov self-assigned this Jul 5, 2022
@pcrespov pcrespov added a:director-v2 issue related with the director-v2 service a:dynamic-sidecar dynamic-sidecar service labels Jul 5, 2022
@codecov
Copy link

codecov bot commented Jul 5, 2022

Codecov Report

Merging #3165 (e0097b6) into master (96e2b00) will decrease coverage by 0.0%.
The diff coverage is 84.6%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3165     +/-   ##
========================================
- Coverage    81.7%   81.7%   -0.1%     
========================================
  Files         723     724      +1     
  Lines       30933   30976     +43     
  Branches     4001    4005      +4     
========================================
+ Hits        25298   25322     +24     
- Misses       4818    4835     +17     
- Partials      817     819      +2     
Flag Coverage Δ
integrationtests 66.3% <70.3%> (+0.1%) ⬆️
unittests 78.2% <83.0%> (-0.1%) ⬇️

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

Impacted Files Coverage Δ
..._director_v2/modules/dynamic_sidecar/_namespace.py 100.0% <ø> (ø)
...r_v2/modules/dynamic_sidecar/api_client/_public.py 90.9% <ø> (-0.1%) ⬇️
...src/simcore_service_dynamic_sidecar/core/errors.py 100.0% <ø> (ø)
...es/dynamic_sidecar/docker_service_specs/sidecar.py 80.8% <36.3%> (+0.4%) ⬆️
...s/models-library/src/models_library/basic_types.py 96.4% <50.0%> (-3.6%) ⬇️
.../service-library/src/servicelib/fastapi/openapi.py 82.9% <60.0%> (-7.3%) ⬇️
...ervice_dynamic_sidecar/api/containers_extension.py 90.1% <83.3%> (+<0.1%) ⬆️
...simcore_service_director_v2/utils/logging_utils.py 86.0% <84.0%> (-0.2%) ⬇️
...ages/service-library/src/servicelib/error_codes.py 84.6% <84.6%> (ø)
...rc/simcore_service_director_v2/core/application.py 90.3% <100.0%> (ø)
... and 14 more

@pcrespov pcrespov force-pushed the is638/dynamic-sidecar-round-5 branch 3 times, most recently from dc0376c to 1e0cb46 Compare July 6, 2022 21:16
@pcrespov pcrespov marked this pull request as ready for review July 6, 2022 22:17
@pcrespov pcrespov requested review from GitHK and sanderegg as code owners July 6, 2022 22:17
@pcrespov pcrespov added this to the Meteora milestone Jul 6, 2022
@pcrespov pcrespov changed the title WIP: ♻️ Is638/dynamic sidecar (round 5) ♻️ Is638/dynamic sidecar (round 5) Jul 7, 2022
@pcrespov pcrespov force-pushed the is638/dynamic-sidecar-round-5 branch from 92d6617 to f61bcd0 Compare July 7, 2022 10:37
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. Thanks.
Please find some minor maybe suggestions.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 7, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@pcrespov pcrespov merged commit 6c97c5e into ITISFoundation:master Jul 7, 2022
@pcrespov pcrespov deleted the is638/dynamic-sidecar-round-5 branch July 7, 2022 15:52
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