Repository: canonical/operator
Pull request #476 (2021-02-10 21:47:07)

Add support for time offset 'Z' (UTC) in pebble._parse_timestamp
Z for +00:00 (UTC) is as per RFC3339 and also what Go produces (see [here](https://play.golang.org/p/yY52SIJH3Yp)). I had just missed it before, because I was testing on NZ time (+13:00).

Add a unit test for this, and also a few for failure cases.

facundobatista: I would not reinvent the wheel, and use a library (that is what libraries are for, right?)

```
>>> import datetime
>>> from dateutil import parser
>>> 
>>> def datetime_nzdt(y, m, d, hour, min, sec, micro=0):
...     tz = datetime.timezone(datetime.timedelta(hours=13))
...     return datetime.datetime(y, m, d, hour, min, sec, micro, tzinfo=tz)
... 
>>> def datetime_utc(y, m, d, hour, min, sec, micro=0):
...     tz = datetime.timezone.utc
...     return datetime.datetime(y, m, d, hour, min, sec, micro, tzinfo=tz)
... 
>>> 
>>> assert parser.parse('2020-12-25T13:45:50+13:00') == datetime_nzdt(2020, 12, 25, 13, 45, 50, 0)
>>> assert parser.parse('2020-12-25T13:45:50.123456789+13:00') == datetime_nzdt(2020, 12, 25, 13, 45, 50, 123456)
>>> assert parser.parse('2021-02-10T04:36:22Z') == datetime_utc(2021, 2, 10, 4, 36, 22, 0)
>>> assert parser.parse('2021-02-10t04:36:22z') == datetime_utc(2021, 2, 10, 4, 36, 22, 0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3/dist-packages/dateutil/parser/_parser.py", line 1356, in parse
    return DEFAULTPARSER.parse(timestr, **kwargs)
  File "/usr/lib/python3/dist-packages/dateutil/parser/_parser.py", line 648, in parse
    raise ValueError("Unknown string format:", timestr)
ValueError: ('Unknown string format:', '2021-02-10t04:36:22z')            <----- this is just a matter of doing .upper()
>>> assert parser.parse('2021-02-10T04:36:22.118970777Z') == datetime_utc(2021, 2, 10, 4, 36, 22, 118970)
>>> assert parser.parse('2020-12-25T13:45:50.123456789+00:00') == datetime_utc(2020, 12, 25, 13, 45, 50, 123456)
>>> tzinfo = datetime.timezone(datetime.timedelta(hours=-11, minutes=-30))
>>> assert parser.parse('2020-12-25T13:45:50.123456789-1130') == datetime.datetime(2020, 12, 25, 13, 45, 50, 123456, tzinfo=tzinfo)
>>> tzinfo = datetime.timezone(datetime.timedelta(hours=4))
>>> assert parser.parse('2000-01-02T03:04:05.006000+04:00') == datetime.datetime(2000, 1, 2, 3, 4, 5, 6000, tzinfo=tzinfo)
```
benhoyt: > I would not reinvent the wheel, and use a library (that is what libraries are for, right?)

@facundobatista I posted about this on #Pebble, and John wants to avoid using `dateutil` here, at least seeing we'd only be using it for this, so as to avoid pulling the lib in for all charms. I kind of agree -- it's not adding a medium-sized dep to every charm, at least not yet, given this would only be used for Pebble.

Patch:
diff --git a/ops/pebble.py b/ops/pebble.py
index d8ea3db..14f6f0a 100644
--- a/ops/pebble.py
+++ b/ops/pebble.py
@@ -67,22 +67,43 @@ class _UnixSocketHandler(urllib.request.AbstractHTTPHandler):
         return self.do_open(_UnixSocketConnection, req, socket_path=self.socket_path)
 
 
-# Matches yyyy-mm-ddTHH:MM:SS.sss[-+]zz(:)zz
+# Matches yyyy-mm-ddTHH:MM:SS(.sss)ZZZ
 _TIMESTAMP_RE = re.compile(
-    r'(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2})\.(\d+)([-+])(\d{2}):?(\d{2})')
+    r'(\d{4})-(\d{2})-(\d{2})[Tt](\d{2}):(\d{2}):(\d{2})(\.\d+)?(.*)')
+
+# Matches [-+]HH:MM
+_TIMEOFFSET_RE = re.compile(r'([-+])(\d{2}):(\d{2})')
 
 
 def _parse_timestamp(s):
-    """Parse timestamp from Go-encoded JSON (which uses 9 decimal places for seconds)."""
+    """Parse timestamp from Go-encoded JSON.
+
+    This parses RFC3339 timestamps (which are a subset of ISO8601 timestamps)
+    that Go's encoding/json package produces for time.Time values.
+
+    Unfortunately we can't use datetime.fromisoformat(), as that does not
+    support more than 6 digits for the fractional second, nor the 'Z' for UTC.
+    Also, it was only introduced in Python 3.7.
+    """
     match = _TIMESTAMP_RE.match(s)
     if not match:
         raise ValueError('invalid timestamp {!r}'.format(s))
-    y, m, d, hh, mm, ss, sub, plus_minus, z1, z2 = match.groups()
-    s = '{y}-{m}-{d}T{hh}:{mm}:{ss}.{sub}{plus_minus}{z1}{z2}'.format(
-        y=y, m=m, d=d, hh=hh, mm=mm, ss=ss, sub=sub[:6],
-        plus_minus=plus_minus, z1=z1, z2=z2,
-    )
-    return datetime.datetime.strptime(s, '%Y-%m-%dT%H:%M:%S.%f%z')
+    y, m, d, hh, mm, ss, sfrac, zone = match.groups()
+
+    if zone in ('Z', 'z'):
+        tz = datetime.timezone.utc
+    else:
+        match = _TIMEOFFSET_RE.match(zone)
+        if not match:
+            raise ValueError('invalid timestamp {!r}'.format(s))
+        sign, zh, zm = match.groups()
+        tz_delta = datetime.timedelta(hours=int(zh), minutes=int(zm))
+        tz = datetime.timezone(tz_delta if sign == '+' else -tz_delta)
+
+    microsecond = round(float(sfrac or '0') * 1000000)
+
+    return datetime.datetime(int(y), int(m), int(d), int(hh), int(mm), int(ss),
+                             microsecond=microsecond, tzinfo=tz)
 
 
 class ServiceError(Exception):


Test patch:
diff --git a/test/test_pebble.py b/test/test_pebble.py
index 74a545f..3060050 100644
--- a/test/test_pebble.py
+++ b/test/test_pebble.py
@@ -40,20 +40,47 @@ def datetime_nzdt(y, m, d, hour, min, sec, micro=0):
 
 class TestHelpers(unittest.TestCase):
     def test_parse_timestamp(self):
+        self.assertEqual(pebble._parse_timestamp('2020-12-25T13:45:50+13:00'),
+                         datetime_nzdt(2020, 12, 25, 13, 45, 50, 0))
+
         self.assertEqual(pebble._parse_timestamp('2020-12-25T13:45:50.123456789+13:00'),
-                         datetime_nzdt(2020, 12, 25, 13, 45, 50, 123456))
+                         datetime_nzdt(2020, 12, 25, 13, 45, 50, 123457))
+
+        self.assertEqual(pebble._parse_timestamp('2021-02-10T04:36:22Z'),
+                         datetime_utc(2021, 2, 10, 4, 36, 22, 0))
+
+        self.assertEqual(pebble._parse_timestamp('2021-02-10t04:36:22z'),
+                         datetime_utc(2021, 2, 10, 4, 36, 22, 0))
+
+        self.assertEqual(pebble._parse_timestamp('2021-02-10T04:36:22.118970777Z'),
+                         datetime_utc(2021, 2, 10, 4, 36, 22, 118971))
 
         self.assertEqual(pebble._parse_timestamp('2020-12-25T13:45:50.123456789+00:00'),
-                         datetime_utc(2020, 12, 25, 13, 45, 50, 123456))
+                         datetime_utc(2020, 12, 25, 13, 45, 50, 123457))
 
         tzinfo = datetime.timezone(datetime.timedelta(hours=-11, minutes=-30))
-        self.assertEqual(pebble._parse_timestamp('2020-12-25T13:45:50.123456789-1130'),
-                         datetime.datetime(2020, 12, 25, 13, 45, 50, 123456, tzinfo=tzinfo))
+        self.assertEqual(pebble._parse_timestamp('2020-12-25T13:45:50.123456789-11:30'),
+                         datetime.datetime(2020, 12, 25, 13, 45, 50, 123457, tzinfo=tzinfo))
 
         tzinfo = datetime.timezone(datetime.timedelta(hours=4))
-        self.assertEqual(pebble._parse_timestamp('2000-01-02T03:04:05.006000+0400'),
+        self.assertEqual(pebble._parse_timestamp('2000-01-02T03:04:05.006000+04:00'),
                          datetime.datetime(2000, 1, 2, 3, 4, 5, 6000, tzinfo=tzinfo))
 
+        with self.assertRaises(ValueError):
+            pebble._parse_timestamp('')
+
+        with self.assertRaises(ValueError):
+            pebble._parse_timestamp('foobar')
+
+        with self.assertRaises(ValueError):
+            pebble._parse_timestamp('2021-99-99T04:36:22Z')
+
+        with self.assertRaises(ValueError):
+            pebble._parse_timestamp(pebble._parse_timestamp('2021-02-10T04:36:22.118970777x'))
+
+        with self.assertRaises(ValueError):
+            pebble._parse_timestamp(pebble._parse_timestamp('2021-02-10T04:36:22.118970777-99:99'))
+
 
 class TestTypes(unittest.TestCase):
     maxDiff = None
@@ -127,7 +154,7 @@ class TestTypes(unittest.TestCase):
         }
         warning = pebble.Warning.from_dict(d)
         self.assertEqual(warning.message, 'Look out...')
-        self.assertEqual(warning.first_added, datetime_nzdt(2020, 12, 25, 17, 18, 54, 16273))
+        self.assertEqual(warning.first_added, datetime_nzdt(2020, 12, 25, 17, 18, 54, 16274))
         self.assertEqual(warning.last_added, datetime_nzdt(2021, 1, 26, 17, 1, 2, 123450))
         self.assertEqual(warning.last_shown, None)
         self.assertEqual(warning.expire_after, '1s')
@@ -214,13 +241,13 @@ class TestTypes(unittest.TestCase):
         self.assertEqual(task.progress.label, '')
         self.assertEqual(task.progress.done, 1)
         self.assertEqual(task.progress.total, 1)
-        self.assertEqual(task.ready_time, datetime_nzdt(2021, 1, 28, 14, 37, 3, 270218))
+        self.assertEqual(task.ready_time, datetime_nzdt(2021, 1, 28, 14, 37, 3, 270219))
         self.assertEqual(task.spawn_time, datetime_nzdt(2021, 1, 28, 14, 37, 2, 247158))
 
         d['ready-time'] = '2021-01-28T14:37:03.270218778+00:00'
         d['spawn-time'] = '2021-01-28T14:37:02.247158162+00:00'
         task = pebble.Task.from_dict(d)
-        self.assertEqual(task.ready_time, datetime_utc(2021, 1, 28, 14, 37, 3, 270218))
+        self.assertEqual(task.ready_time, datetime_utc(2021, 1, 28, 14, 37, 3, 270219))
         self.assertEqual(task.spawn_time, datetime_utc(2021, 1, 28, 14, 37, 2, 247158))
 
     def test_change_id(self):
@@ -266,7 +293,7 @@ class TestTypes(unittest.TestCase):
         self.assertEqual(change.kind, 'autostart')
         self.assertEqual(change.err, 'SILLY')
         self.assertEqual(change.ready, True)
-        self.assertEqual(change.ready_time, datetime_nzdt(2021, 1, 28, 14, 37, 4, 291517))
+        self.assertEqual(change.ready_time, datetime_nzdt(2021, 1, 28, 14, 37, 4, 291518))
         self.assertEqual(change.spawn_time, datetime_nzdt(2021, 1, 28, 14, 37, 2, 247202))
         self.assertEqual(change.status, 'Done')
         self.assertEqual(change.summary, 'Autostart service "svc"')
@@ -275,7 +302,7 @@ class TestTypes(unittest.TestCase):
         d['ready-time'] = '2021-01-28T14:37:04.291517768+00:00'
         d['spawn-time'] = '2021-01-28T14:37:02.247202105+00:00'
         change = pebble.Change.from_dict(d)
-        self.assertEqual(change.ready_time, datetime_utc(2021, 1, 28, 14, 37, 4, 291517))
+        self.assertEqual(change.ready_time, datetime_utc(2021, 1, 28, 14, 37, 4, 291518))
         self.assertEqual(change.spawn_time, datetime_utc(2021, 1, 28, 14, 37, 2, 247202))
 
 
@@ -544,12 +571,12 @@ class TestClient(unittest.TestCase):
         self.assertEqual(change.tasks[0].progress.label, '')
         self.assertEqual(change.tasks[0].progress.total, 1)
         self.assertEqual(change.tasks[0].ready_time,
-                         datetime_nzdt(2021, 1, 28, 14, 37, 3, 270218))
+                         datetime_nzdt(2021, 1, 28, 14, 37, 3, 270219))
         self.assertEqual(change.tasks[0].spawn_time,
                          datetime_nzdt(2021, 1, 28, 14, 37, 2, 247158))
         self.assertEqual(change.ready, True)
         self.assertEqual(change.err, None)
-        self.assertEqual(change.ready_time, datetime_nzdt(2021, 1, 28, 14, 37, 4, 291517))
+        self.assertEqual(change.ready_time, datetime_nzdt(2021, 1, 28, 14, 37, 4, 291518))
         self.assertEqual(change.spawn_time, datetime_nzdt(2021, 1, 28, 14, 37, 2, 247202))
 
     def test_get_changes(self):