🎨 expose `service_run_id` as an env var for both comp and new style dynamic services #6942

GitHK · 2024-12-11T11:22:30Z

What do these changes do?

NOTE: this became very noisy due to a rename and unification of the names

The ${OSPARC_VARIABLE_SERVICE_RUN_ID} will now be replaced in the compose spec with the actual service_run_id.

The service_run_id must be exposed for both comp and dynamic services it's a requirement for some comp services.

Bonus: unified service_run_id under one type ServiceRunID shared by all services

Related issue/s

Vendor Specific Resource tracking (VIP service) osparc-issues#1732
Implement backend for time-based leasing of features with seats (Resource tracking) osparc-issues#1800

How to test

Dev-ops checklist

No ENV changes or I properly updated ENV (read the instruction)

…y-sidecar-run-id

codecov · 2024-12-11T12:39:38Z

Codecov Report

Attention: Patch coverage is 96.87500% with 2 lines in your changes missing coverage. Please review.

Project coverage is 85.32%. Comparing base (acd677a) to head (e726e78).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6942      +/-   ##
==========================================
- Coverage   87.06%   85.32%   -1.74%     
==========================================
  Files        1611     1394     -217     
  Lines       63621    58279    -5342     
  Branches     2026     1637     -389     
==========================================
- Hits        55392    49729    -5663     
- Misses       7895     8279     +384     
+ Partials      334      271      -63

Flag	Coverage Δ
integrationtests	`64.65% <89.28%> (-2.34%)`	⬇️
unittests	`83.96% <95.31%> (-1.39%)`	⬇️

Components	Coverage Δ
api	`∅ <ø> (∅)`
pkg_aws_library	`∅ <ø> (∅)`
pkg_dask_task_models_library	`∅ <ø> (∅)`
pkg_models_library	`91.46% <100.00%> (+0.09%)`	⬆️
pkg_notifications_library	`∅ <ø> (∅)`
pkg_postgres_database	`∅ <ø> (∅)`
pkg_service_integration	`70.02% <ø> (ø)`
pkg_service_library	`74.20% <0.00%> (ø)`
pkg_settings_library	`∅ <ø> (∅)`
pkg_simcore_sdk	`85.38% <ø> (ø)`
agent	`96.82% <100.00%> (ø)`
api_server	`90.13% <ø> (ø)`
autoscaling	`96.09% <ø> (ø)`
catalog	`∅ <ø> (∅)`
clusters_keeper	`99.48% <ø> (ø)`
dask_sidecar	`91.26% <ø> (ø)`
datcore_adapter	`93.18% <ø> (ø)`
director	`76.49% <ø> (+0.08%)`	⬆️
director_v2	`91.43% <93.33%> (ø)`
dynamic_scheduler	`97.10% <ø> (ø)`
dynamic_sidecar	`89.75% <100.00%> (ø)`
efs_guardian	`90.12% <ø> (ø)`
invitations	`93.44% <ø> (ø)`
osparc_gateway_server	`∅ <ø> (∅)`
payments	`92.66% <ø> (ø)`
resource_usage_tracker	`89.62% <100.00%> (+0.30%)`	⬆️
storage	`89.54% <ø> (ø)`
webclient	`∅ <ø> (∅)`
webserver	`79.94% <100.00%> (-4.46%)`	⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update acd677a...e726e78. Read the comment docs.

…ser-services-to-request-dy-sidecar-run-id

…-id' of github.com:GitHK/osparc-simcore-forked into pr-osparc-allow-user-services-to-request-dy-sidecar-run-id

sanderegg

ok, but wrong computational run id

services/director-v2/src/simcore_service_director_v2/modules/comp_scheduler/_scheduler_dask.py

sanderegg

added some comments

services/director-v2/src/simcore_service_director_v2/modules/dask_client.py

services/director-v2/tests/unit/conftest.py

…ser-services-to-request-dy-sidecar-run-id

packages/models-library/src/models_library/services_types.py

matusdrobuliak66

I’m fine with this unification 👍, but are we 100% sure that the concept of a run ID is the same in all three use cases: computation, dynamic, and resource usage tracker?

sanderegg

Good for me but please check my comments about the naming, and at least use import RunID as ServiceRunID in all the computational related modules in dv-2.

services/director-v2/src/simcore_service_director_v2/modules/comp_scheduler/_scheduler_base.py

services/director-v2/src/simcore_service_director_v2/modules/comp_scheduler/_scheduler_dask.py

services/director-v2/src/simcore_service_director_v2/modules/dask_client.py

services/director-v2/src/simcore_service_director_v2/utils/dask.py

services/director-v2/tests/unit/with_dbs/test_utils_dask.py

services/director-v2/tests/unit/test_modules_dask_client.py

pcrespov

please reassign back when unblocked. thx

…ser-services-to-request-dy-sidecar-run-id

sonarcloud · 2024-12-17T11:59:08Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

pcrespov

thx

pcrespov · 2024-12-17T17:18:55Z

packages/models-library/src/models_library/services_types.py

+from .users import UserID
+
+if TYPE_CHECKING:
+    from .projects import ProjectID


I wonder why this this necessary? Is it because of cyclic dependencies?

TIP: For things like ProjectID we can always move these to common_library. I already did that with user_enums and groups_enums (check how it is imported in the Groups and Users models)

pcrespov · 2024-12-17T17:24:15Z

services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/substitutions.py

@@ -120,6 +121,7 @@ def create(cls, app: FastAPI):
            ("OSPARC_VARIABLE_NODE_ID", "node_id"),
            ("OSPARC_VARIABLE_PRODUCT_NAME", "product_name"),
            ("OSPARC_VARIABLE_STUDY_UUID", "project_id"),
+            ("OSPARC_VARIABLE_SERVICE_RUN_ID", "run_id"),


ok.
TIP: did you know that now you can check the new registered osparc variable using the CLI?

$ cd services/director-v2 $ make install-dev $ simcore-service osparc-variables OSPARC_VARIABLE_API_HOST OSPARC_VARIABLE_API_KEY OSPARC_VARIABLE_API_SECRET OSPARC_VARIABLE_NODE_ID OSPARC_VARIABLE_PRODUCT_NAME OSPARC_VARIABLE_STUDY_UUID OSPARC_VARIABLE_USER_EMAIL OSPARC_VARIABLE_USER_ID OSPARC_VARIABLE_USER_ROLE

sanderegg

thanks!

sanderegg · 2024-12-17T20:47:34Z

packages/models-library/src/models_library/services_types.py

@@ -44,12 +54,15 @@ class RunID(str):
    and old volumes for different runs.
    Avoids overwriting data that left dropped on the node (due to an error)
    and gives the osparc-agent an opportunity to back it up.
+    The resource-usage-tracker tracker uses these RunIDs to keep track of
+    resource usage from comp and dynamic services.


Suggested change

resource usage from comp and dynamic services.

resource usage from computational and dynamic services.

sanderegg · 2024-12-17T20:49:00Z

packages/models-library/src/models_library/services_types.py

@@ -68,7 +92,7 @@ def __get_pydantic_core_schema__(
        return core_schema.no_info_after_validator_function(cls, handler(str))

    @classmethod
-    def validate(cls, v: "RunID | str", _: ValidationInfo) -> "RunID":
+    def validate(cls, v: "ServiceRunID | str", _: ValidationInfo) -> "ServiceRunID":


Q: should this not raise a ValueError instead of TypeError?

run_id can now be requested as an env var on request by a service

24ff119

GitHK self-assigned this Dec 11, 2024

GitHK added the a:director-v2 issue related with the director-v2 service label Dec 11, 2024

GitHK added this to the Event Horizon milestone Dec 11, 2024

Merge branch 'master' into pr-osparc-allow-user-services-to-request-d…

df34a54

…y-sidecar-run-id

Andrei Neagu added 3 commits December 12, 2024 16:34

Merge remote-tracking branch 'upstream/master' into pr-osparc-allow-u…

47c4eb8

…ser-services-to-request-dy-sidecar-run-id

added context for run_id in comp tasks

daccfdc

Merge branch 'pr-osparc-allow-user-services-to-request-dy-sidecar-run…

8d9b796

…-id' of github.com:GitHK/osparc-simcore-forked into pr-osparc-allow-user-services-to-request-dy-sidecar-run-id

GitHK changed the title ~~🎨 dynamic-service can now request the run id at runtime in their env vars~~ 🎨 expose run_id as an env var for both comp and new style dynamic services Dec 13, 2024

added fixture and refactor tests

d6b89a9

GitHK marked this pull request as ready for review December 13, 2024 07:43

GitHK requested review from sanderegg and pcrespov as code owners December 13, 2024 07:43

GitHK requested review from giancarloromeo, mguidon and matusdrobuliak66 December 13, 2024 07:43

Andrei Neagu added 2 commits December 13, 2024 08:56

fixed broken tests

4204990

moved fixture where it is used

ff3298c

sanderegg requested changes Dec 13, 2024

View reviewed changes

services/director-v2/src/simcore_service_director_v2/modules/comp_scheduler/_scheduler_dask.py Outdated Show resolved Hide resolved

sanderegg reviewed Dec 13, 2024

View reviewed changes

services/director-v2/src/simcore_service_director_v2/modules/dask_client.py Outdated Show resolved Hide resolved

services/director-v2/tests/unit/conftest.py Outdated Show resolved Hide resolved

Andrei Neagu added 3 commits December 13, 2024 09:44

refactor with proper values

2d1b08e

refactor

13d56de

fixed test

dfdacbb

GitHK requested a review from sanderegg December 13, 2024 08:49

Andrei Neagu added 5 commits December 13, 2024 10:13

refactor to use new RunID

486fb33

refactor types

978f407

fixed type

0cb9f1f

repalced

af0bed3

fixed imports

a72b6b5

Merge remote-tracking branch 'upstream/master' into pr-osparc-allow-u…

98de753

…ser-services-to-request-dy-sidecar-run-id

matusdrobuliak66 reviewed Dec 13, 2024

View reviewed changes

packages/models-library/src/models_library/services_types.py Outdated Show resolved Hide resolved

matusdrobuliak66 approved these changes Dec 13, 2024

View reviewed changes

sanderegg reviewed Dec 13, 2024

View reviewed changes

pcrespov reviewed Dec 13, 2024

View reviewed changes

Andrei Neagu added 3 commits December 16, 2024 14:55

Merge remote-tracking branch 'upstream/master' into pr-osparc-allow-u…

b0d715c

…ser-services-to-request-dy-sidecar-run-id

renamed run_id to service_run_id and RunID to ServiceRunID

5b67d46

rename

e9be2ee

GitHK changed the title ~~🎨 expose run_id as an env var for both comp and new style dynamic services~~ 🎨 expose service_run_id as an env var for both comp and new style dynamic services Dec 16, 2024

Andrei Neagu added 2 commits December 16, 2024 15:38

refactor

40ac24e

fixed naming

fe7ee55

giancarloromeo approved these changes Dec 17, 2024

View reviewed changes

Andrei Neagu added 2 commits December 17, 2024 12:55

Merge remote-tracking branch 'upstream/master' into pr-osparc-allow-u…

2ded5fe

…ser-services-to-request-dy-sidecar-run-id

fixed failing test

e726e78

GitHK requested review from sanderegg and pcrespov December 17, 2024 11:58

pcrespov approved these changes Dec 17, 2024

View reviewed changes

sanderegg approved these changes Dec 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎨 expose `service_run_id` as an env var for both comp and new style dynamic services #6942

🎨 expose `service_run_id` as an env var for both comp and new style dynamic services #6942

GitHK commented Dec 11, 2024 •

edited

Loading

codecov bot commented Dec 11, 2024 •

edited

Loading

sanderegg left a comment

sanderegg left a comment

matusdrobuliak66 left a comment

sanderegg left a comment

pcrespov left a comment

sonarcloud bot commented Dec 17, 2024

pcrespov left a comment

pcrespov Dec 17, 2024

pcrespov Dec 17, 2024

sanderegg left a comment

sanderegg Dec 17, 2024

sanderegg Dec 17, 2024

	resource usage from comp and dynamic services.
	resource usage from computational and dynamic services.

🎨 expose service_run_id as an env var for both comp and new style dynamic services #6942

Are you sure you want to change the base?

🎨 expose service_run_id as an env var for both comp and new style dynamic services #6942

Conversation

GitHK commented Dec 11, 2024 • edited Loading

What do these changes do?

Related issue/s

How to test

Dev-ops checklist

codecov bot commented Dec 11, 2024 • edited Loading

Codecov Report

sanderegg left a comment

Choose a reason for hiding this comment

sanderegg left a comment

Choose a reason for hiding this comment

matusdrobuliak66 left a comment

Choose a reason for hiding this comment

sanderegg left a comment

Choose a reason for hiding this comment

pcrespov left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Dec 17, 2024

Quality Gate passed

pcrespov left a comment

Choose a reason for hiding this comment

pcrespov Dec 17, 2024

Choose a reason for hiding this comment

pcrespov Dec 17, 2024

Choose a reason for hiding this comment

sanderegg left a comment

Choose a reason for hiding this comment

sanderegg Dec 17, 2024

Choose a reason for hiding this comment

sanderegg Dec 17, 2024

Choose a reason for hiding this comment

🎨 expose `service_run_id` as an env var for both comp and new style dynamic services #6942

🎨 expose `service_run_id` as an env var for both comp and new style dynamic services #6942

GitHK commented Dec 11, 2024 •

edited

Loading

codecov bot commented Dec 11, 2024 •

edited

Loading