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

🐛Api-server wrong start/stop/submit timestamps #4254

Merged
merged 26 commits into from
May 22, 2023

Conversation

sanderegg
Copy link
Member

@sanderegg sanderegg commented May 19, 2023

What do these changes do?

fixes #4231

timestamp BEFORE AFTER
submitted request local now time last modification of pipeline
started_at timestamp of inspect request when pipeline state was started (very unreliable) start timestamp when director-v2 submits the job to the comp. backend
stopped_at timestamp of inspect request when pipeline state was stopped (very unreliable and tends to be the latest time it was called) stop timestamp when director-v2 receives pipeline completed (abort, fail or success)

image

Bonuses:

  • improves testing by using pydantic models to detect missing variables
  • some SQLAlchemy 2.0 updates

Related issue/s

How to test

DevOps Checklist

@sanderegg sanderegg added the a:apiserver api-server service label May 19, 2023
@sanderegg sanderegg added this to the Pastel de Nata milestone May 19, 2023
@sanderegg sanderegg self-assigned this May 19, 2023
@codecov
Copy link

codecov bot commented May 19, 2023

Codecov Report

Merging #4254 (216887a) into master (b13a831) will decrease coverage by 0.2%.
The diff coverage is 92.1%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #4254     +/-   ##
========================================
- Coverage    82.6%   82.5%   -0.2%     
========================================
  Files         957     957             
  Lines       41505   41542     +37     
  Branches      963     963             
========================================
- Hits        34300   34275     -25     
- Misses       6984    7059     +75     
+ Partials      221     208     -13     
Flag Coverage Δ
integrationtests ?
unittests 82.5% <92.1%> (+0.5%) ⬆️

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

Impacted Files Coverage Δ
.../simcore_service_webserver/projects/projects_db.py 96.2% <ø> (ø)
...bserver/studies_dispatcher/_projects_permalinks.py 100.0% <ø> (ø)
...ore_service_director_v2/api/routes/computations.py 65.9% <20.0%> (-22.5%) ⬇️
...rector_v2/modules/comp_scheduler/base_scheduler.py 91.9% <60.0%> (+7.5%) ⬆️
...c/simcore_service_director_v2/api/routes/health.py 83.3% <66.6%> (-2.4%) ⬇️
..._director_v2/modules/db/repositories/comp_tasks.py 92.9% <92.8%> (-1.9%) ⬇️
...ls-library/src/models_library/projects_pipeline.py 100.0% <100.0%> (ø)
...sdk/src/simcore_sdk/node_ports_common/dbmanager.py 42.4% <100.0%> (-43.7%) ⬇️
.../simcore_service_api_server/models/schemas/jobs.py 96.4% <100.0%> (+1.5%) ⬆️
...e_api_server/utils/solver_job_models_converters.py 100.0% <100.0%> (+2.6%) ⬆️
... and 8 more

... and 73 files with indirect coverage changes

@sanderegg sanderegg marked this pull request as ready for review May 22, 2023 07:29
@sanderegg sanderegg requested a review from mguidon May 22, 2023 07:30
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.

Excellent! Thx for the addition!

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.

👍

@sanderegg sanderegg force-pushed the bugfix/api-timestamps-wrong branch from 3ea46f8 to 216887a Compare May 22, 2023 12:17
@sonarcloud
Copy link

sonarcloud bot commented May 22, 2023

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

@codeclimate
Copy link

codeclimate bot commented May 22, 2023

Code Climate has analyzed commit 216887a and detected 0 issues on this pull request.

View more on Code Climate.

@sanderegg sanderegg merged commit 3ff6e30 into ITISFoundation:master May 22, 2023
@sanderegg sanderegg deleted the bugfix/api-timestamps-wrong branch May 22, 2023 13:30
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request May 30, 2023
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:apiserver api-server service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API: Timestamps are wrong
3 participants