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

✨ Store computational services progress in comp_tasks table, decouple webserver from api-server 🗃️ #4197

Merged

Conversation

sanderegg
Copy link
Member

@sanderegg sanderegg commented May 5, 2023

What do these changes do?

  • add progress column to comp_tasks table
  • adapt director-v2 to store last progress in comp_tasks table
  • adapt director-v2 to return the progress from comp_tasks table
  • adapt director-v2 to return overall pipeline progress (based on pipeline each node progress)
  • dask-sidecar: uncontrolled progress value from comp. services gets forcibly set between 0 and 1
  • api-server now returns progress as well (PublicAPI: Correctly retrieve progress of jobs #2444)

NOTE: Next PR should then remove the progress changes in the DB done by the webserver. This should completely decouple the webserver from the api-server run pipelines.

Bonus:

  • fix flaky with migration service (remove service instead of killing the task which would trigger a double restart)
  • some sqlalchemy 2.0 refactoring (use Row._asdict() to convert to a dict)

Related issue/s

How to test

DevOps Checklist

@sanderegg sanderegg added the a:director-v2 issue related with the director-v2 service label May 5, 2023
@sanderegg sanderegg added this to the Pastel de Nata milestone May 5, 2023
@sanderegg sanderegg self-assigned this May 5, 2023
@codecov
Copy link

codecov bot commented May 5, 2023

Codecov Report

Merging #4197 (07b4d8d) into master (4cee078) will increase coverage by 2.5%.
The diff coverage is 97.6%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #4197      +/-   ##
=========================================
+ Coverage    84.0%   86.6%    +2.5%     
=========================================
  Files         956     558     -398     
  Lines       41489   27527   -13962     
  Branches      960     547     -413     
=========================================
- Hits        34884   23842   -11042     
+ Misses       6400    3554    -2846     
+ Partials      205     131      -74     
Flag Coverage Δ
integrationtests 63.6% <91.0%> (+2.6%) ⬆️
unittests 84.7% <91.7%> (+2.3%) ⬆️

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

Impacted Files Coverage Δ
...ector_v2/modules/db/repositories/comp_pipelines.py 94.2% <ø> (ø)
...e_director_v2/modules/db/repositories/comp_runs.py 94.8% <ø> (ø)
...ce_director_v2/modules/db/repositories/projects.py 100.0% <ø> (ø)
...or_v2/modules/db/repositories/projects_networks.py 52.1% <0.0%> (ø)
..._director_v2/modules/db/repositories/comp_tasks.py 96.9% <92.3%> (+0.2%) ⬆️
...brary/src/servicelib/long_running_tasks/_models.py 97.0% <100.0%> (+<0.1%) ⬆️
...e_service_director_v2/models/domains/comp_tasks.py 100.0% <100.0%> (ø)
...rector_v2/modules/comp_scheduler/base_scheduler.py 91.9% <100.0%> (ø)
...rector_v2/modules/comp_scheduler/dask_scheduler.py 93.8% <100.0%> (+<0.1%) ⬆️
...ce_director_v2/modules/db/repositories/clusters.py 94.7% <100.0%> (+<0.1%) ⬆️
... and 2 more

... and 430 files with indirect coverage changes

@sanderegg sanderegg force-pushed the comp/add_progress_comp_tasks branch 7 times, most recently from bcce9c1 to 6efdbea Compare May 11, 2023 16:33
@sanderegg sanderegg changed the title ✨ store task progress in comp_tasks table ✨ store task progress in comp_tasks table 🗃️ May 11, 2023
@sanderegg sanderegg force-pushed the comp/add_progress_comp_tasks branch 3 times, most recently from 9a013b7 to feec870 Compare May 16, 2023 19:32
@sanderegg sanderegg marked this pull request as ready for review May 16, 2023 19:32
@sanderegg sanderegg changed the title ✨ store task progress in comp_tasks table 🗃️ ✨ Store computational services progress in comp_tasks table, decouple webserver from api-server 🗃️ May 16, 2023
@sanderegg sanderegg requested review from mguidon and odeimaiz May 16, 2023 19:34
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.

Great work!
Please check test_solvers_jobs_api.py check on the progress ... that seems suspicious to me

services/director-v2/setup.cfg Outdated Show resolved Hide resolved
tests/public-api/test_solvers_jobs_api.py Outdated Show resolved Hide resolved
Copy link
Contributor

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

👍

@sanderegg sanderegg enabled auto-merge (squash) May 17, 2023 07:18
@sanderegg sanderegg force-pushed the comp/add_progress_comp_tasks branch from ba7aca1 to 6c89d2b Compare May 17, 2023 08:21
@sanderegg sanderegg force-pushed the comp/add_progress_comp_tasks branch from 6c89d2b to 07b4d8d Compare May 17, 2023 09:47
@codeclimate
Copy link

codeclimate bot commented May 17, 2023

Code Climate has analyzed commit 07b4d8d and detected 0 issues on this pull request.

View more on Code Climate.

@sonarqubecloud
Copy link

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

@sanderegg sanderegg merged commit e2ef814 into ITISFoundation:master May 17, 2023
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request May 30, 2023
24 tasks
@sanderegg sanderegg deleted the comp/add_progress_comp_tasks branch June 8, 2023 15:12
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PublicAPI: Correctly retrieve progress of jobs
4 participants