Fix.6701.scheduler time zone change bug

changes.d/6719.fix.md

@@ -0,0 +1 @@
+Fix incorrect job submission time reported after a time zone change.

cylc/flow/wallclock.py

@@ -17,6 +17,7 @@
 
 from calendar import timegm
 from datetime import datetime, timedelta, timezone
+from typing import Dict, Optional
 
 from metomi.isodatetime.timezone import (
     get_local_time_zone_format, get_local_time_zone, TimeZoneFormatMode)
@@ -37,21 +38,16 @@
 TIME_FORMAT_EXTENDED = "%H:%M:%S"
 TIME_FORMAT_EXTENDED_SUB_SECOND = "%H:%M:%S.%f"
 
-TIME_ZONE_STRING_LOCAL_BASIC = get_local_time_zone_format(
-    TimeZoneFormatMode.reduced)
-TIME_ZONE_STRING_LOCAL_EXTENDED = get_local_time_zone_format(
-    TimeZoneFormatMode.extended)
 TIME_ZONE_STRING_UTC = "Z"
 TIME_ZONE_UTC_UTC_OFFSET = (0, 0)
-TIME_ZONE_LOCAL_UTC_OFFSET = get_local_time_zone()
-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 = {
-    "hours": TIME_ZONE_LOCAL_UTC_OFFSET[0],
-    "minutes": TIME_ZONE_LOCAL_UTC_OFFSET[1],
-    "string_basic": TIME_ZONE_STRING_LOCAL_BASIC,
-    "string_extended": TIME_ZONE_STRING_LOCAL_EXTENDED
+    "hours": get_local_time_zone()[0],
+    "minutes": get_local_time_zone()[1],
+    "string_basic": get_local_time_zone_format(
+        TimeZoneFormatMode.reduced),
+    "string_extended": get_local_time_zone_format(
+        TimeZoneFormatMode.extended)
 }
 
 TIME_ZONE_UTC_INFO = {
@@ -113,34 +109,49 @@ def get_current_time_string(display_sub_seconds=False, override_use_utc=None,
                            use_basic_format=use_basic_format)
 
 
-def get_time_string(date_time, display_sub_seconds=False,
-                    override_use_utc=None, use_basic_format=False,
-                    date_time_is_local=False, custom_time_zone_info=None):
+def get_time_string(
+    date_time: datetime,
+    display_sub_seconds: bool = False,
+    override_use_utc: Optional[bool] = None,
+    use_basic_format: bool = False,
+    date_time_is_local: bool = False,
+    custom_time_zone_info: Optional[Dict] = None,
+):
     """Return a string representing the current system time.
 
-    Arguments:
-    date_time - a datetime.datetime object.
-
-    Keyword arguments:
-    display_sub_seconds (default False) - a boolean that, if True,
-    switches on microsecond reporting
-    override_use_utc (default None) - a boolean (or None) that, if
-    True, switches on utc time zone reporting. If False, it switches
-    off utc time zone reporting (even if _FLAGS['utc_mode'] is True). If None,
-    the _FLAGS['utc_mode'] boolean is used.
-    use_basic_format (default False) - a boolean that, if True,
-    represents the date/time without "-" or ":" delimiters. This is
-    most useful for filenames where ":" may cause problems.
-    date_time_is_local - a boolean that, if True, indicates that
-    the date_time argument object is in the local time zone, not UTC.
-    custom_time_zone_info (default None) - a dictionary that enforces
-    a particular time zone. It looks like {"hours": _hours,
-    "minutes": _minutes, "string": _string} where _hours and _minutes
-    are the hours and minutes offset from UTC and _string is the string
-    to use as the time zone designator.
+    Args:
+        date_time: Datetime to operate on.
+        display_sub_seconds:
+            Switch on microsecond reporting.
+        override_use_utc:
+            Switch on utc time zone reporting.
+            If False, it switches off utc time zone reporting even
+            if ``_FLAGS['utc_mode']`` is True).
+            If None, the ``_FLAGS['utc_mode']`` boolean is used.
+        use_basic_format:
+            Represent the date/time without "-" or ":"
+            delimiters. This is useful for filenames, where ":" may
+            cause problems.
+        date_time_is_local:
+            Indicates that the date_time argument
+            object is in the local time zone, not UTC.
+        custom_time_zone_info:
+            A dictionary that enforces a particular time zone:
+
+            .. code-block:: python
+                {
+                    "hours": _hours,           # offset from UTC
+                    "minutes": _minutes,       # offset from utc
+                    "string_basic": _string,    # timezone designators
+                    "string_extened": _string
+                }
+
+            Usage of ``string_basic`` or ``string_extended`` is
+            switched by ``use_basic_format``.
 
     """
     time_zone_string = None
+    local_tz = get_local_time_zone()
     if custom_time_zone_info is not None:
         custom_hours = custom_time_zone_info["hours"]
         custom_minutes = custom_time_zone_info["minutes"]
@@ -149,8 +160,7 @@ def get_time_string(date_time, display_sub_seconds=False,
         else:
             custom_string = custom_time_zone_info["string_extended"]
         if date_time_is_local:
-            date_time_hours = TIME_ZONE_LOCAL_UTC_OFFSET_HOURS
-            date_time_minutes = TIME_ZONE_LOCAL_UTC_OFFSET_MINUTES
+            date_time_hours, date_time_minutes = local_tz
         else:
             date_time_hours, date_time_minutes = (0, 0)
         diff_hours = custom_hours - date_time_hours
@@ -162,17 +172,18 @@ def get_time_string(date_time, display_sub_seconds=False,
         time_zone_string = TIME_ZONE_STRING_UTC
         if date_time_is_local:
             date_time = date_time - timedelta(
-                hours=TIME_ZONE_LOCAL_UTC_OFFSET_HOURS,
-                minutes=TIME_ZONE_LOCAL_UTC_OFFSET_MINUTES
+                hours=local_tz[0],
+                minutes=local_tz[1]
             )
     else:
         if use_basic_format:
-            time_zone_string = TIME_ZONE_STRING_LOCAL_BASIC
+            time_zone_string = get_local_time_zone_format(
+                TimeZoneFormatMode.reduced)
         else:
-            time_zone_string = TIME_ZONE_STRING_LOCAL_EXTENDED
+            time_zone_string = get_local_time_zone_format(
+                TimeZoneFormatMode.extended)
         if not date_time_is_local:
-            diff_hours = TIME_ZONE_LOCAL_UTC_OFFSET_HOURS
-            diff_minutes = TIME_ZONE_LOCAL_UTC_OFFSET_MINUTES
+            diff_hours, diff_minutes = local_tz
             date_time = date_time + timedelta(
                 hours=diff_hours, minutes=diff_minutes)
     if use_basic_format:

tests/conftest.py

@@ -18,6 +18,7 @@
 import re
 from pathlib import Path
 from shutil import rmtree
+import time
 from typing import List, Optional, Tuple
 
 import pytest
@@ -202,3 +203,21 @@ def _call(*args, **kwargs):
         return calls
 
     return _capcall
+
+
+@pytest.fixture
+def set_timezone(monkeypatch):
+    """Fixture to temporarily set a timezone.
+
+    Will use a very implausible timezone if none is provided.
+    """
+    def patch(time_zone: str = 'XXX-19:17'):
+        monkeypatch.setenv('TZ', time_zone)
+        time.tzset()
+
+    try:
+        yield patch
+    finally:
+        # Reset to the original time zone after the test
+        monkeypatch.undo()
+        time.tzset()

tests/integration/test_workflow_db_mgr.py

@@ -14,6 +14,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+from datetime import datetime, timedelta
 import pytest
 import sqlite3
 from typing import TYPE_CHECKING
@@ -178,3 +179,32 @@ async def test_record_only_non_clock_triggers(
     assert db_select(schd, False, 'xtriggers', 'signature') == [
         ('xrandom(100)',),
         ('xrandom(100, _=Not a real wall clock trigger)',)]
+
+
+async def test_time_zone_writing(
+    one_conf,
+    flow,
+    scheduler,
+    start,
+    db_select,
+    set_timezone
+):
+    """Don't store scheduler startup timezone forever.
+
+    https://github.com/cylc/cylc-flow/issues/6701
+    """
+    set_timezone('XXX-19:00')
+    schd = scheduler(flow(one_conf), paused_start=False, run_mode='live')
+    async with start(schd):
+        itask = schd.pool.get_tasks()[0]
+        now = datetime.now().astimezone()
+        set_timezone('XXX-19:17')
+        schd.submit_task_jobs([itask])
+
+    # Check the db time_submit:
+    (time_submit,) = db_select(schd, False, 'task_jobs', 'time_submit')[0]
+    time_submit = datetime.strptime(time_submit, '%Y-%m-%dT%H:%M:%S%z')
+    # The submit time should be approx correct:
+    assert (
+        abs(time_submit - now) < timedelta(seconds=10)
+    ), f"{time_submit} ~= {now}"

tests/unit/test_wallclock.py

@@ -14,10 +14,16 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+from datetime import datetime
 import pytest
+from pytest import param
 
 from metomi.isodatetime.data import CALENDAR
-from cylc.flow.wallclock import get_unix_time_from_time_string
+from cylc.flow.wallclock import (
+    get_current_time_string,
+    get_time_string,
+    get_unix_time_from_time_string,
+)
 
 
 @pytest.mark.parametrize(
@@ -60,3 +66,75 @@ def test_get_unix_time_from_time_string_360(time_str, time_sec):
 def test_get_unix_time_from_time_string_error(value, error):
     with pytest.raises(error):
         get_unix_time_from_time_string(value)
+
+
+def test_get_current_time_string(set_timezone):
+    """It reacts to local time zone changes.
+
+    https://github.com/cylc/cylc-flow/issues/6701
+    """
+    set_timezone()
+    res = get_current_time_string()
+    assert res[-6:] == '+19:17'
+
+
+@pytest.mark.parametrize(
+    'arg, kwargs, expect',
+    (
+        param(
+            datetime(2000, 12, 13, 15, 30, 12, 123456),
+            {},
+            '2000-12-14T10:47:12+19:17',
+            id='good',
+        ),
+        param(
+            datetime(2000, 12, 13, 15, 30, 12, 123456),
+            {'date_time_is_local': True},
+            '2000-12-13T15:30:12+19:17',
+            id='dt_is_local',
+        ),
+        param(
+            datetime(2000, 12, 13, 15, 30, 12, 123456),
+            {
+                'custom_time_zone_info': {
+                    'hours': 0,
+                    'minutes': -20,
+                    'string_basic': 'XXX+00:20',
+                },
+                'use_basic_format': True
+            },
+            '20001213T151012XXX+00:20',
+            id='custom_time_zone_info_string_basic',
+        ),
+        param(
+            datetime(2000, 12, 13, 15, 30, 12, 123456),
+            {
+                'custom_time_zone_info': {
+                    'hours': 0,
+                    'minutes': -20,
+                    'string_extended': ':UK/Exeter',
+                },
+                'use_basic_format': False
+            },
+            '2000-12-13T15:10:12:UK/Exeter',
+            id='custom_time_zone_info_string_extended',
+        ),
+        param(
+            datetime(2000, 12, 13, 15, 30, 12, 123456),
+            {
+                'custom_time_zone_info': {
+                    'hours': 0,
+                    'minutes': -20,
+                    'string_extended': ':UK/Exeter',
+                },
+                'use_basic_format': False,
+                'date_time_is_local': True,
+            },
+            '2000-12-12T19:53:12:UK/Exeter',
+            id='date_time_is_local',
+        ),
+    ),
+)
+def test_get_time_string(set_timezone, arg, kwargs, expect):
+    set_timezone()
+    assert get_time_string(arg, **kwargs) == expect