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

✨adds cli command to monitor study status #3401

Merged
merged 4 commits into from
Oct 3, 2022

Conversation

GitHK
Copy link
Contributor

@GitHK GitHK commented Sep 30, 2022

What do these changes do?

Monitoring the status of very large projects (20+ dynamic services) is very hard and current tooling is not sufficient.

These changes add a cli option added to director-v2 to observe the status of dynamic services in a project, as seen by the backend.

How to use

From inside director-v2 run simcore-service-director-v2 project-state PROJECT_ID and a self updating
table showing the status of all services will be shown. See below for more details:

$ simcore-service-director-v2 project-state --help
Usage: simcore-service-director-v2 project-state [OPTIONS] PROJECT_ID

  Displays the state of the nodes in the project.

Arguments:
  PROJECT_ID  [required]

Options:
  --blocking / --no-blocking  [default: blocking]
  --update-interval INTEGER   [default: 5]
  --help                      Show this message and exit.
  • --blocking mode is enabled, the cli will not return and display a self updating window, with --no-blocking it returns immediately
  • --update-interval is te rate at which the services status is queried

Sep-30-2022 14-47-26

Related issue/s

How to test

Checklist

  • Unit tests for the changes exist
  • Runs in the swarm

@GitHK GitHK self-assigned this Sep 30, 2022
@GitHK GitHK added this to the vaporwave milestone Sep 30, 2022
@codecov
Copy link

codecov bot commented Sep 30, 2022

Codecov Report

Merging #3401 (75a7b43) into master (5fd5898) will increase coverage by 1.1%.
The diff coverage is 85.5%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3401     +/-   ##
========================================
+ Coverage    81.9%   83.0%   +1.1%     
========================================
  Files         738     818     +80     
  Lines       31319   34680   +3361     
  Branches      848    1363    +515     
========================================
+ Hits        25673   28811   +3138     
- Misses       5497    5688    +191     
- Partials      149     181     +32     
Flag Coverage Δ
integrationtests 68.2% <0.0%> (-0.5%) ⬇️
unittests 79.9% <85.5%> (+1.4%) ⬆️

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

Impacted Files Coverage Δ
...s/service-library/src/servicelib/services_utils.py 0.0% <0.0%> (ø)
...or-v2/src/simcore_service_director_v2/cli/_core.py 87.9% <87.9%> (ø)
...v2/src/simcore_service_director_v2/cli/__init__.py 100.0% <100.0%> (ø)
...log/src/simcore_service_catalog/api/routes/dags.py 62.7% <0.0%> (-11.7%) ⬇️
...simcore_service_director_v2/modules/node_rights.py 98.1% <0.0%> (-1.0%) ⬇️
...ector_v2/modules/dynamic_sidecar/scheduler/task.py 83.4% <0.0%> (-0.8%) ⬇️
...imcore_service_webserver/garbage_collector_core.py 69.8% <0.0%> (-0.7%) ⬇️
...tor_v2/modules/dynamic_sidecar/docker_api/_core.py 96.0% <0.0%> (-0.5%) ⬇️
...vice_datcore_adapter/api/dependencies/pennsieve.py 100.0% <0.0%> (ø)
...s/models-library/src/models_library/services_ui.py 100.0% <0.0%> (ø)
... and 78 more

@GitHK GitHK marked this pull request as ready for review September 30, 2022 13:03
@GitHK GitHK changed the title ✨cli option to monitor study status from ✨adds cli command to monitor study status Sep 30, 2022
Copy link
Member

@mrnicegyu11 mrnicegyu11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

supercool, thanks a lot!

Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some minors

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.

Great, thanks for the very nice CLI. Please have a look at the comments

nodes_failed_to_save: list[NodeIDStr] = []
for node_uuid, node_content in project_at_db.workbench.items():
# onl dynamic-sidecars are used
if not await requires_dynamic_sidecar(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: do we have studies with legacy services? if yes what of them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we still have some of there here and there. They are being deprecated at the moment, so I expect them to be rolled out.

async with _initialized_app() as app:
dynamic_sidecar_client = api_client.get_dynamic_sidecar_client(app)
await _save_node_state(
app, dynamic_sidecar_client, retry_save, NodeIDStr(f"{node_id}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: no way to get the node label beforehand?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No not in this case, I do not want you to provide the project_id just to fetch a label here. It is not worth the effort.

) -> Optional[DynamicServiceOut]:
try:
result = await client.get(
f"http://localhost:8000/v2/dynamic_services/{node_uuid}", # NOSONAR
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: why is this calling itself via the REST API? why not use directly the fct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a different process. For convenience you are just launching it from the same container. This CLI command only had the database connection configured nothing else.

@sonarcloud
Copy link

sonarcloud bot commented Oct 3, 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.2% 0.2% Duplication

@GitHK GitHK merged commit e584132 into ITISFoundation:master Oct 3, 2022
@GitHK GitHK deleted the pr-osparc-cli-monitor branch October 3, 2022 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants