Merge pull request #82 from thread/clarify-time-trigger

Clarify time trigger

Author: aaron7

routemaster/config/__init__.py

@@ -9,7 +9,6 @@
     Webhook,
     FeedConfig,
     NextStates,
-    TimeTrigger,
     NoNextStates,
     StateMachine,
     DatabaseConfig,
@@ -18,8 +17,11 @@
     MetadataTrigger,
     ConstantNextState,
     ContextNextStates,
+    SystemTimeTrigger,
     LoggingPluginConfig,
+    TimezoneAwareTrigger,
     ContextNextStatesOption,
+    MetadataTimezoneAwareTrigger,
 )
 from routemaster.config.loader import load_config, load_database_config
 from routemaster.config.exceptions import ConfigError
@@ -36,7 +38,6 @@
     'FeedConfig',
     'NextStates',
     'ConfigError',
-    'TimeTrigger',
     'NoNextStates',
     'StateMachine',
     'DatabaseConfig',
@@ -45,6 +46,9 @@
     'MetadataTrigger',
     'ConstantNextState',
     'ContextNextStates',
+    'SystemTimeTrigger',
     'LoggingPluginConfig',
+    'TimezoneAwareTrigger',
     'ContextNextStatesOption',
+    'MetadataTimezoneAwareTrigger',
 )

routemaster/config/loader.py

@@ -3,13 +3,14 @@
 import os
 import re
 import datetime
-from typing import Any, Dict, List, Iterable, Optional
+from typing import Any, Dict, List, Union, Iterable, Optional
 
 import yaml
 import jsonschema
 import pkg_resources
 import jsonschema.exceptions
 
+from routemaster.timezones import get_known_timezones
 from routemaster.text_utils import join_comma_or
 from routemaster.config.model import (
     Gate,
@@ -20,7 +21,6 @@
     Webhook,
     FeedConfig,
     NextStates,
-    TimeTrigger,
     NoNextStates,
     StateMachine,
     DatabaseConfig,
@@ -29,8 +29,11 @@
     MetadataTrigger,
     ConstantNextState,
     ContextNextStates,
+    SystemTimeTrigger,
     LoggingPluginConfig,
+    TimezoneAwareTrigger,
     ContextNextStatesOption,
+    MetadataTimezoneAwareTrigger,
 )
 from routemaster.exit_conditions import ExitConditionProgram
 from routemaster.config.exceptions import ConfigError
@@ -249,11 +252,6 @@ def _load_gate(path: Path, yaml_state: Yaml, feed_names: List[str]) -> Gate:
 
 
 def _load_trigger(path: Path, yaml_trigger: Yaml) -> Trigger:
-    if len(yaml_trigger.keys()) > 1:  # pragma: no branch
-        raise ConfigError(  # pragma: no cover
-            f"Trigger at path {'.'.join(path)} cannot be of multiple types.",
-        )
-
     if 'time' in yaml_trigger:
         return _load_time_trigger(path, yaml_trigger)
     elif 'metadata' in yaml_trigger:
@@ -269,7 +267,22 @@ def _load_trigger(path: Path, yaml_trigger: Yaml) -> Trigger:
         )
 
 
-def _load_time_trigger(path: Path, yaml_trigger: Yaml) -> TimeTrigger:
+def _validate_known_timezone(path: Path, timezone: str) -> None:
+    if timezone not in get_known_timezones():
+        raise ConfigError(
+            f"Timezone '{timezone}' at path {'.'.join(path)} is not a known "
+            f"timezone.",
+        )
+
+
+def _load_time_trigger(
+    path: Path,
+    yaml_trigger: Yaml,
+) -> Union[
+    SystemTimeTrigger,
+    TimezoneAwareTrigger,
+    MetadataTimezoneAwareTrigger,
+]:
     format_ = '%Hh%Mm'
     try:
         dt = datetime.datetime.strptime(str(yaml_trigger['time']), format_)
@@ -279,7 +292,21 @@ def _load_time_trigger(path: Path, yaml_trigger: Yaml) -> TimeTrigger:
             f"Time trigger '{yaml_trigger['time']}' at path {'.'.join(path)} "
             f"does not meet expected format: {format_}.",
         ) from None
-    return TimeTrigger(time=trigger)
+
+    if 'timezone' in yaml_trigger:
+        timezone_path = path + ['timezone']
+        timezone: str = yaml_trigger['timezone']
+        if timezone.startswith('metadata.'):
+            _validate_context_lookups(timezone_path, [timezone], [])
+            return MetadataTimezoneAwareTrigger(
+                time=trigger,
+                timezone_metadata_path=timezone.split('.')[1:],
+            )
+        else:
+            _validate_known_timezone(timezone_path, timezone)
+            return TimezoneAwareTrigger(time=trigger, timezone=timezone)
+
+    return SystemTimeTrigger(time=trigger)
 
 
 RE_INTERVAL = re.compile(

routemaster/config/model.py

@@ -22,9 +22,37 @@
     from routemaster.context import Context  # noqa
 
 
-class TimeTrigger(NamedTuple):
-    """Time based trigger for exit condition evaluation."""
+class SystemTimeTrigger(NamedTuple):
+    """
+    System time based trigger for exit condition evaluation.
+
+    This trigger runs at the given time according to the system on which
+    routemaster is running.
+    """
+    time: datetime.time
+
+
+class TimezoneAwareTrigger(NamedTuple):
+    """
+    Fixed timezone aware trigger for exit condition evaluation.
+
+    This trigger runs at the time according to the named timezone. The timezone
+    should be spelled using an IANA name, for example: 'Europe/London'.
+    """
+    time: datetime.time
+    timezone: str
+
+
+class MetadataTimezoneAwareTrigger(NamedTuple):
+    """
+    Metadata timezone aware trigger for exit condition evaluation.
+
+    This trigger uses a label's metadata to determine the current timezone and
+    otherwise behaves like a `TimezoneAwareTrigger`. The timezone should be
+    spelled using an IANA name, for example: 'Europe/London'.
+    """
     time: datetime.time
+    timezone_metadata_path: Sequence[str]
 
 
 class IntervalTrigger(NamedTuple):
@@ -53,7 +81,14 @@ class OnEntryTrigger:
     """Trigger on entry to a given gate."""
 
 
-Trigger = Union[TimeTrigger, IntervalTrigger, MetadataTrigger, OnEntryTrigger]
+Trigger = Union[
+    SystemTimeTrigger,
+    TimezoneAwareTrigger,
+    MetadataTimezoneAwareTrigger,
+    IntervalTrigger,
+    MetadataTrigger,
+    OnEntryTrigger,
+]
 
 
 class ConstantNextState(NamedTuple):

routemaster/config/schema.yaml

@@ -68,6 +68,9 @@ properties:
                             time:
                               type: string
                               pattern: '^[0-9]{1,2}h[0-9]{2}m$'
+                            timezone:
+                              type: string
+                              pattern: '^([a-zA-Z]+/[a-zA-Z_]+|metadata\.\S+)$'
                           required:
                             - time
                           additionalProperties: false

routemaster/config/tests/test_loading.py

@@ -15,7 +15,6 @@
     Webhook,
     FeedConfig,
     ConfigError,
-    TimeTrigger,
     NoNextStates,
     StateMachine,
     DatabaseConfig,
@@ -24,8 +23,11 @@
     MetadataTrigger,
     ConstantNextState,
     ContextNextStates,
+    SystemTimeTrigger,
     LoggingPluginConfig,
+    TimezoneAwareTrigger,
     ContextNextStatesOption,
+    MetadataTimezoneAwareTrigger,
     load_config,
 )
 from routemaster.exit_conditions import ExitConditionProgram
@@ -99,7 +101,15 @@ def test_realistic_config():
                     Gate(
                         name='start',
                         triggers=[
-                            TimeTrigger(time=datetime.time(18, 30)),
+                            SystemTimeTrigger(time=datetime.time(18, 30)),
+                            TimezoneAwareTrigger(
+                                time=datetime.time(12, 25),
+                                timezone='Europe/London',
+                            ),
+                            MetadataTimezoneAwareTrigger(
+                                time=datetime.time(13, 37),
+                                timezone_metadata_path=['timezone'],
+                            ),
                             MetadataTrigger(metadata_path='foo.bar'),
                             IntervalTrigger(
                                 interval=datetime.timedelta(hours=1),
@@ -228,6 +238,11 @@ def test_raises_for_invalid_time_format_in_trigger():
         load_config(yaml_data('trigger_time_format_invalid'))
 
 
+def test_raises_for_invalid_timezone_name_in_trigger():
+    with assert_config_error("Could not validate config file against schema."):
+        load_config(yaml_data('trigger_timezone_name_invalid'))
+
+
 def test_raises_for_invalid_path_format_in_trigger():
     with assert_config_error("Could not validate config file against schema."):
         load_config(yaml_data('path_format_context_trigger_invalid'))
@@ -306,7 +321,15 @@ def test_environment_variables_override_config_file_for_database_config():
                     Gate(
                         name='start',
                         triggers=[
-                            TimeTrigger(time=datetime.time(18, 30)),
+                            SystemTimeTrigger(time=datetime.time(18, 30)),
+                            TimezoneAwareTrigger(
+                                time=datetime.time(12, 25),
+                                timezone='Europe/London',
+                            ),
+                            MetadataTimezoneAwareTrigger(
+                                time=datetime.time(13, 37),
+                                timezone_metadata_path=['timezone'],
+                            ),
                             MetadataTrigger(metadata_path='foo.bar'),
                             IntervalTrigger(
                                 interval=datetime.timedelta(hours=1),

routemaster/cron.py

@@ -4,7 +4,7 @@
 import functools
 import itertools
 import threading
-from typing import List, Callable, Iterable
+from typing import Callable, Iterable
 
 import schedule
 from typing_extensions import Protocol
@@ -14,27 +14,29 @@
     Gate,
     State,
     Action,
-    TimeTrigger,
     StateMachine,
     IntervalTrigger,
     MetadataTrigger,
+    SystemTimeTrigger,
+    TimezoneAwareTrigger,
+    MetadataTimezoneAwareTrigger,
 )
 from routemaster.state_machine import (
+    LabelProvider,
     LabelStateProcessor,
     process_cron,
     process_gate,
     process_action,
     labels_in_state,
     labels_needing_metadata_update_retry_in_gate,
 )
+from routemaster.cron_processors import (
+    TimezoneAwareProcessor,
+    MetadataTimezoneAwareProcessor,
+)
 
 IsTerminating = Callable[[], bool]
 
-# Note: This function will be called in a different transaction to where we
-# iterate over the results, so to prevent confusion or the possible
-# introduction of errors, we require all the data up-front.
-LabelProvider = Callable[[App, StateMachine, State], List[str]]
-
 
 class CronProcessor(Protocol):
     """Type signature for the cron processor callable."""
@@ -118,14 +120,30 @@ def _configure_schedule_for_state(
         )
     elif isinstance(state, Gate):
         for trigger in state.triggers:
-            if isinstance(trigger, TimeTrigger):
+            if isinstance(trigger, SystemTimeTrigger):
                 scheduler.every().day.at(
                     f"{trigger.time.hour:02d}:{trigger.time.minute:02d}",
                 ).do(
                     processor,
                     fn=process_gate,
                     label_provider=labels_in_state,
                 )
+            elif isinstance(trigger, TimezoneAwareTrigger):
+                func = functools.partial(
+                    processor,
+                    fn=process_gate,
+                    label_provider=labels_in_state,
+                )
+                scheduler.every().minute.do(
+                    TimezoneAwareProcessor(func, trigger),
+                )
+            elif isinstance(trigger, MetadataTimezoneAwareTrigger):
+                scheduler.every().minute.do(
+                    MetadataTimezoneAwareProcessor(
+                        functools.partial(processor, fn=process_gate),
+                        trigger,
+                    ),
+                )
             elif isinstance(trigger, IntervalTrigger):
                 scheduler.every(
                     trigger.interval.total_seconds(),