Fix.6701.scheduler time zone change bug#6719

Merged

hjoliver merged 5 commits intocylc:8.6.xfrom

wxtim:fix.6701.Scheduler_time_zone_change_bug

Dec 9, 2025

Member

wxtim commented Apr 8, 2025 •

edited

Loading

Closes #6701

The first time the scheduler loads cylc.flow.wallclock the constants TIME_ZONE_STRING_LOCAL_BASIC and TIME_ZONE_STRING_LOCAL_EXTENDED are populated for the lifetime of the scheduler. This means that the time zone returned by get_time_string will remain the same, even if the time it adds to the time zone label changes time zone.

For example a scheduler started in UK winter will always return date_time.strftime(date_time_format_string) + 'Z'. If the clocks subsequently go forward, the time recorded for local time 09:00+01:00 will be 09:00Z which is equivelent to 10:00+01:00. The consequence is that the database table will record the time_submit field as nearly an hour after the time_submit_exit field.

This bug is particularly noticable in Cylc Review where the following was observed:

i.e. the task has submitted 22 minutes in the future!

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).
Changelog entry included if this is a change that can affect users
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.

wxtim requested review from MetRonnie and hjoliver

April 8, 2025 13:10

wxtim force-pushed the fix.6701.Scheduler_time_zone_change_bug branch from 6a49f26 to 2ad10fe Compare

April 8, 2025 13:13

wxtim commented

View reviewed changes

cylc/flow/wallclock.py

-              TIME_ZONE_LOCAL_UTC_OFFSET_HOURS = TIME_ZONE_LOCAL_UTC_OFFSET[0]
-              TIME_ZONE_LOCAL_UTC_OFFSET_MINUTES = TIME_ZONE_LOCAL_UTC_OFFSET[1]
               TIME_ZONE_LOCAL_INFO = {

Member Author

wxtim Apr 8, 2025

This is only used in cylc.flow.data_store_mgr.DataStoreMgr.generate_definition_elements which seems to be for fields which are not updated. @dwsutherland might wish to comment on whether this safe.

Member

MetRonnie Apr 9, 2025 •

edited

Loading

I can't think why this information is needed in the data store/GQL API at all.

Member

dwsutherland May 6, 2025 •

edited

Loading

Yes, the time zone is recorded on the workflow object from TIME_ZONE_LOCAL_INFO:

cylc-flow/cylc/flow/data_store_mgr.py

Line 697 in d5ac2e2

if get_utc_mode():

That will only be reread from TIME_ZONE_LOCAL_INFO on start/restart and reload...
Does TIME_ZONE_LOCAL_INFO also get updated on reload?

The job nodes just get passed the time by the events or job manager.

Member Author

wxtim May 6, 2025 •

edited

Loading

That leads to a real possibility that the time zones in the GUI will be wrong on DST change? I think I should create an "investigate this" ticket.

Member

MetRonnie May 6, 2025

What is the use case for having in the GraphQL API the local system time zone on scheduler start (except not when the workflow is running in UTC mode)? Note this is not the cycle point time zone

oliver-sanders assigned wxtim

oliver-sanders added this to the 8.4.x milestone

oliver-sanders added the bug label

Member

MetRonnie commented Apr 9, 2025

N.B. will conflict with #6640

MetRonnie reviewed

View reviewed changes

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

tests/unit/test_wallclock.py Outdated Show resolved Hide resolved

cylc/flow/wallclock.py

-              TIME_ZONE_LOCAL_UTC_OFFSET_HOURS = TIME_ZONE_LOCAL_UTC_OFFSET[0]
-              TIME_ZONE_LOCAL_UTC_OFFSET_MINUTES = TIME_ZONE_LOCAL_UTC_OFFSET[1]
               TIME_ZONE_LOCAL_INFO = {

Member

MetRonnie Apr 9, 2025 •

edited

Loading

I can't think why this information is needed in the data store/GQL API at all.

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

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

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

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

Member

MetRonnie commented Apr 9, 2025

@wxtim Can you please explain the bug in the PR description

wxtim requested a review from MetRonnie

April 10, 2025 14:34

MetRonnie reviewed

View reviewed changes

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

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

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

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

changes.d/6719.fix.md Outdated Show resolved Hide resolved

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

tests/unit/test_wallclock.py Outdated Show resolved Hide resolved

wxtim requested a review from MetRonnie

April 11, 2025 10:26

MetRonnie reviewed

View reviewed changes

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

MetRonnie linked an issue

that may be closed by this pull request

UTC Edge effect #6701

Closed

MetRonnie modified the milestones: 8.4.x, 8.4.3

wxtim force-pushed the fix.6701.Scheduler_time_zone_change_bug branch 2 times, most recently from f055f41 to 56be9d1 Compare

April 15, 2025 13:27

wxtim requested a review from MetRonnie

April 15, 2025 13:28

wxtim commented

View reviewed changes

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

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

MetRonnie reviewed

View reviewed changes

tests/conftest.py Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

Member Author

wxtim commented Apr 16, 2025

Credit to @MetRonnie for working out why Mac was behaving differently: Python on Mac doesn't appear to require time.tzset() to update the value of TZ in Python. This means that when the orginal fixture using monkeypatch.context exited from the patched context, Python returned to the original TZ, rather than remaining in the alternate.

MetRonnie reviewed

View reviewed changes

tests/conftest.py Outdated Show resolved Hide resolved

MetRonnie reviewed

View reviewed changes

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

wxtim force-pushed the fix.6701.Scheduler_time_zone_change_bug branch from ccaf3be to 1bc013f Compare

April 17, 2025 08:57

Member Author

wxtim commented Apr 17, 2025

@hjoliver - Review Poke

Member Author

wxtim commented May 8, 2025

@hjoliver Review poke

oliver-sanders modified the milestones: 8.4.3, 8.4.x

MetRonnie removed their request for review

July 16, 2025 11:47

MetRonnie force-pushed the 8.4.x branch from bba6f72 to 970df19 Compare

July 22, 2025 09:05

Member Author

wxtim commented Aug 14, 2025

@hjoliver Review poke

MetRonnie modified the milestones: 8.4.x, 8.5.x

oliver-sanders modified the milestones: 8.5.x, 8.6.x

oliver-sanders changed the base branch from 8.4.x to 8.6.x

October 2, 2025 08:51

oliver-sanders added this to the 8.6.x milestone

wxtim force-pushed the fix.6701.Scheduler_time_zone_change_bug branch 3 times, most recently from fa4bfef to 6384348 Compare

December 9, 2025 11:17

wxtim and others added 4 commits

December 9, 2025 11:23


          Tests for failures caused by caching time zones on first starting

2c7eebd

Cylc.


          Fix issues with timezones

01dc57a

response to review

rewrote test in a less resource intensive way

make the set_tz fixture more generic


          * Improve unit testing of cylc/flow/wallclock.py

9970dc8

* Modernise docstring.


          Apply suggestions from code review

08379ac

Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>

wxtim force-pushed the fix.6701.Scheduler_time_zone_change_bug branch from 6384348 to 08379ac Compare

December 9, 2025 11:25


          fix lint error from rebase

4ac813f

MetRonnie approved these changes

View reviewed changes

hjoliver approved these changes

View reviewed changes

Member

hjoliver left a comment

LGTM, sorry for the long delay @wxtim

hjoliver merged commit 52d2559 into cylc:8.6.x

23 checks passed

wxtim deleted the fix.6701.Scheduler_time_zone_change_bug branch

December 10, 2025 09:17

MetRonnie mentioned this pull request

Address some warnings #6640

Merged

6 tasks

MetRonnie modified the milestones: 8.6.x, 8.6.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug