ID tokeniser bug

[hjoliver]

This is a valid ISO8601 format: 2050-01-01T00:00Z but the tokeniser cuts the :00Z bit off the end, which ends up with the wrong result if your system clock is not UTC or if you have non-zero minutes.

>>> from cylc.flow.id import Tokens

>>> Tokens('foo//20500101T0000Z')
<id: foo//20500101T0000Z>  # good

>>> Tokens('foo//2050-01-01T00:00Z')
<id: foo//2050-01-01T00>  # bug - later this gets interpreted as local time

We disallow this a cycle point format in workflows because : is not a valid character in filenames, but cycle points can be specified on the command line too and then converted to the workflow format.

We have a functional test that does this, and fails for non-UTC clocks: tests/f/workflow-state/06a-noformat.t (for the cylc workflow-state command).

This diff seems to fix the tokeniser (don't disallow ':' in cycle point):

$ git diff
diff --git a/cylc/flow/id.py b/cylc/flow/id.py
index cba3c4833..b9d7cc2e1 100644
--- a/cylc/flow/id.py
+++ b/cylc/flow/id.py
@@ -403,10 +403,10 @@ class Tokens(dict):
 # //cycle[:sel][/task[:sel][/job[:sel]]]
 RELATIVE_PATTERN = rf'''
     //
-    (?P<{IDTokens.Cycle.value}>[^~\/:\n]+)
+    (?P<{IDTokens.Cycle.value}>[^~\/\n]+)
     (?:
       :
-      (?P<{IDTokens.Cycle.value}_sel>[^\/:\n]+)
+      (?P<{IDTokens.Cycle.value}_sel>[^\/\n]+)
     )?
     (?:
       /

Alternatively we might want to detect the : here and raise an error instead, but I can't see any harm in allowing this user-friendly format in CLI and UI queries?

[oliver-sanders]

Should be ok to apply that diff.

[hjoliver]

Actually, the second part of my diff is probably wrong (cycle selector), but made me think:

Does the selector delimiter (:) prevent us from supporting this datetime format on the CLI as well? wflow//*:running, wflow//2010:running ...

[oliver-sanders]

Oh dammit, of course, no we cannot support : in cycle points :(

I don't think it had occurred to me that it was even possible to do so.

It would be kinda possible to hack this in along the lines of your diff above, but really nasty (you would have to attempt to parse the cycle+selector to tell if together they form a valid cycle point, then implement special handling)

[hjoliver]

Let's just disallow that format, across the board.

This still remains a bug though, because the CLI currently accepts the format but handles it badly.

[wxtim]

@oliver-sanders - are you happy for me to block this cycle point format on the CLI?

[oliver-sanders]

It already is as the result of this issue. No need for further action.

[MetRonnie]

It already is as the result of this issue. No need for further action.

It is not blocked though, it leads to the command failing to match tasks.

$ cylc hold horse/pamphlet/run1//2012-01-01T00:00Z/foo
Command submitted; the scheduler will log any problems.

INFO - [20120101T0000Z/foo/01:submitted] => running
...
INFO - Command "hold" received. ID=ff867448-fba3-4870-9978-06ec94e9fb7f
    hold(tasks=['2012-01-01T00:00Z/foo'])
WARNING - No active tasks matching: 2012-01-01T00:00Z/foo