Ensure that UUID is retrieved from database on workflow restart. #5623

wxtim · 2023-07-10T10:54:49Z

Check List

I have read CONTRIBUTING.md and added my name as a Code Contributor.
Contains logically grouped changes (else tidy your branch by rebase).
Does not contain off-topic changes (use other PRs for other changes).
Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
Tests are included (or explain why tests are not needed).
~~CHANGES.md entry included if this is a change that can affect users~~ Not currently a user facing change.
~~Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.~~
If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

oliver-sanders · 2023-07-10T11:03:10Z

It's a little more complex than this. The UUID is already being loaded on restart:

Lines 1330 to 1332 in 6b4055d

    
           elif key == self.workflow_db_mgr.KEY_UUID_STR: 
        
               self.uuid_str = value 
        
               LOG.info('+ workflow UUID = %s', value)

But it's not happening at the right time (search for .uuid_str) e.g:

cylc-flow/cylc/flow/scheduler.py

Lines 386 to 407 in 6b4055d

    
           self.task_events_mgr = TaskEventsManager( 
        
               self.workflow, 
        
               self.proc_pool, 
        
               self.workflow_db_mgr, 
        
               self.broadcast_mgr, 
        
               self.xtrigger_mgr, 
        
               self.data_store_mgr, 
        
               self.options.log_timestamp, 
        
               self.bad_hosts, 
        
               self.reset_inactivity_timer 
        
           ) 
        
           self.task_events_mgr.uuid_str = self.uuid_str 
        
           self.task_job_mgr = TaskJobManager( 
        
               self.workflow, 
        
               self.proc_pool, 
        
               self.workflow_db_mgr, 
        
               self.task_events_mgr, 
        
               self.data_store_mgr, 
        
               self.bad_hosts 
        
           ) 
        
           self.task_job_mgr.task_remote_mgr.uuid_str = self.uuid_str

To close #5615, we need to remove the UUID which gets set in Scheduler.__init__ (only required on first run) and ensure that the UUID gets set on the Scheduler before it gets accessed.

This is kinda similar to the TaskProxy.platform bug where having an erroneous default set by __init__ has covered over runtime errors disguising a problem until we spotted behaviour differences.

cylc/flow/scheduler.py

tests/integration/test_scheduler.py

MetRonnie

Looks good, I have some suggestions at wxtim#56

- Set Scheduler UUID at same place regardless of restart or not - Fix references to "uuid_str" -> "uuid" in docs - Tidy

cylc/flow/scheduler.py

tests/integration/test_scheduler.py

Co-authored-by: Oliver Sanders <oliver.sanders@metoffice.gov.uk>

…lc into fix.get_uuid_from_db_on_restart * 'fix.get_uuid_from_db_on_restart' of github.com:wxtim/cylc: Update tests/integration/test_scheduler.py

MetRonnie · 2023-07-25T11:00:37Z

Not currently a user facing change.

It is user-facing in that the contact file is user facing and the UUID is printed to the log. However most users won't care about this so I guess a changelog entry isn't needed?

oliver-sanders

👍

cylc/flow/scheduler.py

Co-authored-by: Oliver Sanders <oliver.sanders@metoffice.gov.uk>

wxtim · 2023-08-01T08:33:50Z

@MetRonnie - It's three weeks since you approved - want to have a double check?

MetRonnie · 2023-08-02T14:07:43Z

Can you get the tests to run? The last commit included [skip ci] in the description so either add a changelog commit or change the description and force push

…db_on_restart * upstream/8.2.x: Update etc/bin/swarm GH Actions: attempt FF merge for sync GH Actions: always create separate sync branch Fix style Tweak swarm configure. Changelog Towncrier: run draft build as part of fast tests Bump dev version (cylc#5637) Prepare release 8.2.0 Use towncrier for changelog generation Fix changelog Tidy CLI help text cylc clean remote re-invocation: don't scan for workflows

wxtim · 2023-08-03T06:48:45Z

Can you get the tests to run? The last commit included [skip ci] in the description so either add a changelog commit or change the description and force push

Yes, barring CodeCov.

wxtim requested review from MetRonnie and oliver-sanders July 10, 2023 10:55

wxtim self-assigned this Jul 10, 2023

wxtim added bug Something is wrong :( small labels Jul 10, 2023

wxtim added this to the cylc-8.2.0 milestone Jul 10, 2023

MetRonnie reviewed Jul 10, 2023

View reviewed changes

cylc/flow/scheduler.py Outdated Show resolved Hide resolved

cylc/flow/scheduler.py Outdated Show resolved Hide resolved

oliver-sanders reviewed Jul 10, 2023

View reviewed changes

tests/integration/test_scheduler.py Outdated Show resolved Hide resolved

wxtim marked this pull request as draft July 13, 2023 09:15

wxtim force-pushed the fix.get_uuid_from_db_on_restart branch from c27ce7b to ed0d228 Compare July 13, 2023 12:32

move setting of UUID on restart

cfa06fe

wxtim force-pushed the fix.get_uuid_from_db_on_restart branch from 8f599f8 to cfa06fe Compare July 13, 2023 12:58

test fix

522c74a

wxtim marked this pull request as ready for review July 13, 2023 15:28

wxtim requested review from oliver-sanders and MetRonnie July 13, 2023 15:28

MetRonnie reviewed Jul 14, 2023

View reviewed changes

UUID followup (#56)

c9e2a30

- Set Scheduler UUID at same place regardless of restart or not - Fix references to "uuid_str" -> "uuid" in docs - Tidy

MetRonnie approved these changes Jul 14, 2023

View reviewed changes

Merge branch 'master' into fix.get_uuid_from_db_on_restart

7b27827

wxtim modified the milestones: cylc-8.2.0, cylc-8.2.1 Jul 19, 2023

oliver-sanders reviewed Jul 24, 2023

View reviewed changes

cylc/flow/scheduler.py Outdated Show resolved Hide resolved

tests/integration/test_scheduler.py Outdated Show resolved Hide resolved

wxtim and others added 3 commits July 25, 2023 09:01

Update tests/integration/test_scheduler.py

958736d

Co-authored-by: Oliver Sanders <oliver.sanders@metoffice.gov.uk>

.

d95772b

Merge branch 'fix.get_uuid_from_db_on_restart' of github.com:wxtim/cy…

c809880

…lc into fix.get_uuid_from_db_on_restart * 'fix.get_uuid_from_db_on_restart' of github.com:wxtim/cylc: Update tests/integration/test_scheduler.py

MetRonnie changed the base branch from master to 8.2.x July 25, 2023 16:02

wxtim requested a review from oliver-sanders July 26, 2023 09:47

wxtim marked this pull request as draft July 26, 2023 10:02

wxtim added 3 commits July 26, 2023 12:38

Load workflow parameters into memory early in the startup process

fb8aa8b

mypyfix

ebe5d80

don't create an un-needed db

f11a304

wxtim marked this pull request as ready for review July 26, 2023 13:55

oliver-sanders approved these changes Jul 28, 2023

View reviewed changes

cylc/flow/scheduler.py Outdated Show resolved Hide resolved

cylc/flow/scheduler.py Outdated Show resolved Hide resolved

cylc/flow/scheduler.py Outdated Show resolved Hide resolved

_parse_workflow_params → _set_workflow_params

c7f554c

Co-authored-by: Oliver Sanders <oliver.sanders@metoffice.gov.uk>

wxtim force-pushed the fix.get_uuid_from_db_on_restart branch 2 times, most recently from 275576e to 43f6c31 Compare August 1, 2023 08:31

wxtim force-pushed the fix.get_uuid_from_db_on_restart branch from 43f6c31 to e1a7440 Compare August 3, 2023 06:17

MetRonnie approved these changes Aug 3, 2023

View reviewed changes

MetRonnie merged commit 6c3beff into cylc:8.2.x Aug 3, 2023
25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure that UUID is retrieved from database on workflow restart. #5623

Ensure that UUID is retrieved from database on workflow restart. #5623

wxtim commented Jul 10, 2023 •

edited

Loading

oliver-sanders commented Jul 10, 2023 •

edited

Loading

MetRonnie left a comment

MetRonnie commented Jul 25, 2023

oliver-sanders left a comment

wxtim commented Aug 1, 2023

MetRonnie commented Aug 2, 2023

wxtim commented Aug 3, 2023

Ensure that UUID is retrieved from database on workflow restart. #5623

Ensure that UUID is retrieved from database on workflow restart. #5623

Conversation

wxtim commented Jul 10, 2023 • edited Loading

oliver-sanders commented Jul 10, 2023 • edited Loading

MetRonnie left a comment

Choose a reason for hiding this comment

MetRonnie commented Jul 25, 2023

oliver-sanders left a comment

Choose a reason for hiding this comment

wxtim commented Aug 1, 2023

MetRonnie commented Aug 2, 2023

wxtim commented Aug 3, 2023

wxtim commented Jul 10, 2023 •

edited

Loading

oliver-sanders commented Jul 10, 2023 •

edited

Loading