Xarthisius commented 3 years ago

If job ended up in a success, and had a correct number of steps defined by progressTotal, it was actually impossible to issue final wt_progress message if both job status and progress were updated in the same step.

NOTE I think that logic is still wrong in case we have an intermediate job finishing and the subsequent sub-job has one step...

Requires: https://github.com/whole-tale/girder/pull/13 or prebuilt image: wholetale/girder:event

Xarthisius commented 3 years ago

I think it's ready. I ended up rewriting updateNotifications to use a different kind of event (jobs.job.update) which contains both current state of the job along with progress parameters that are going to change.

codecov[bot] commented 3 years ago

Codecov Report

Merging #488 (ea5d433) into master (7f658ca) will increase coverage by 0.02%. The diff coverage is 96.66%.

@@            Coverage Diff             @@
##           master     #488      +/-   ##
==========================================
+ Coverage   93.11%   93.13%   +0.02%     
==========================================
  Files          52       52              
  Lines        3948     3960      +12     
==========================================
+ Hits         3676     3688      +12     
  Misses        272      272

Impacted Files	Coverage Δ
server/models/tale.py	`96.98% <ø> (ø)`
server/__init__.py	`93.18% <96.00%> (+0.17%)`	:arrow_up:
server/utils.py	`93.02% <100.00%> (+0.71%)`	:arrow_up:

Continue to review full report at Codecov.

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

whole-tale / girder_wholetale

Correctly update wt_progress for last notification #488

Codecov Report