Arvados

The original PR updates the ciso8601 library to >=2.0.0 for sdk/python. Note that we also have this dependency for services/fuse and sdk/cwl which need to be updated.

Note the ciso8601 migration guide regarding the change to parse_datetime_unaware now called parse_datetime_naive: https://github.com/closeio/ciso8601/blob/master/CHANGELOG.md#parse_datetime_unaware-has-been-renamed

Confirm that when we are using parse_datetime_naive we are parsing datetimes with time zone info into naive datetimes, instead of just normal parsing.

Another change which subsequently breaks a sdk/python test: https://github.com/closeio/ciso8601/blob/master/CHANGELOG.md#other-changes

======================================================================
ERROR: test_get_trash_at (tests.test_collections.CollectionMethods)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/src/arvados/sdk/python/tests/test_collections.py", line 817, in test_get_trash_at
self.assertEqual(c.get_trash_at(), ciso8601.parse_datetime('2111-01-01T11:11:11.111111000Z'))
File "/usr/src/arvados/sdk/python/arvados/collection.py", line 1351, in get_trash_at
return ciso8601.parse_datetime(self._api_response["trash_at"])
ImportError: Cannot parse a timestamp with time zone information without the pytz dependency. Install it with `pip install pytz`.

Apparently our datetime strings have a 'Z' at the end, which is considered time zone info by ciso8601 according to
https://pypi.org/project/ciso8601/#time-zone-information.

Consequently, we would need a pytz dependency when using ciso8601 2.0 with python 2.7 according to https://github.com/closeio/ciso8601/blob/master/CHANGELOG.md#other-changes.

A few notes about different ciso8601 parsing options using the following script with Python 2.7:

import ciso8601

tmp = ciso8601.parse_xxxxxx('2111-01-01T11:11:11.111111000Z')
print(str(tmp))

• ImportError: Cannot parse a timestamp with time zone information without the pytz dependency.

• Works, but you lose all time zone info i.e. 2111-01-01 11:11:11.111111

• ImportError: Cannot parse a timestamp with time zone information without the pytz dependency.

An additional caveat is that parse_rfc3339 requires a UTC offset in the string and throws an exception if its not there. In Python 3, no pytz is required because python datetime is used under the hood instead, so both parse_datetime and parse_rfc3339 produce 2111-01-01 11:11:11.111111+00:00 when the results are converted back to strings.

The easiest approach is to create a conditional Python 2.7 pytz dependency for sdk/python, sdk/cwl, and services/fuse. I think this is reasonable. Here is a little background on why pytz isn't an explicit dependency for ciso8601 https://github.com/closeio/ciso8601#dependency-on-pytz-python-2.

If we want to tighten our adherence to rfc3339, we could also switch to using parse_rfc3339; but note that this also requires a conditional Python 2.7 dependency.

Latest at d9e5d620d8e68b349bb79b9749a74110603a89d4

- Changed usage of parse_datetime_as_naive (formerly parse_datetime_unaware) to parse_datetime
- Added conditional pytz dependency for Python 2.7 for sdk/python, sdk/cwl, and services/fuse.

Unit tests: https://ci.curoverse.com/view/Developer/job/developer-run-tests/1094/
Conformance and integration tests: https://ci.curoverse.com/view/CWL/job/arvados-cwl-conformance-tests/47/

Eric Biagiotti wrote:

Latest at d9e5d620d8e68b349bb79b9749a74110603a89d4

- Changed usage of parse_datetime_as_naive (formerly parse_datetime_unaware) to parse_datetime
- Added conditional pytz dependency for Python 2.7 for sdk/python, sdk/cwl, and services/fuse.

Unit tests: https://ci.curoverse.com/view/Developer/job/developer-run-tests/1094/
Conformance and integration tests: https://ci.curoverse.com/view/CWL/job/arvados-cwl-conformance-tests/47/

This LGTM

Peter Amstutz wrote:

[...]

Reverted back to parse_datetime_as_naive in 2e4ec565fc74e6de19fefd2a8670fd319544ce73

Most of our python code uses datetime.utcnow(), which is time zone naive. In order to be compatible with the removal of as_naive in ffef38539dd36e224abb75f0480cff1deb319124 we would need to update all these instances and introduce the pytz dependency for all versions of python. This seems unnecessary considering the original scope of this story and the fact that we don't currently have other problems related to datetimes.

Unit Tests: https://ci.curoverse.com/view/Developer/job/developer-run-tests/1099/
C&I Tests: https://ci.curoverse.com/view/CWL/job/arvados-cwl-conformance-tests/52/

Rerun of fuse tests: https://ci.curoverse.com/job/developer-run-tests-services-fuse/1118/

Eric Biagiotti wrote:

Peter Amstutz wrote:

[...]

Reverted back to parse_datetime_as_naive in 2e4ec565fc74e6de19fefd2a8670fd319544ce73

Most of our python code uses datetime.utcnow(), which is time zone naive. In order to be compatible with the removal of as_naive in ffef38539dd36e224abb75f0480cff1deb319124 we would need to update all these instances and introduce the pytz dependency for all versions of python. This seems unnecessary considering the original scope of this story and the fact that we don't currently have other problems related to datetimes.

Unit Tests: https://ci.curoverse.com/view/Developer/job/developer-run-tests/1099/
C&I Tests: https://ci.curoverse.com/view/CWL/job/arvados-cwl-conformance-tests/52/

Rerun of fuse tests: https://ci.curoverse.com/job/developer-run-tests-services-fuse/1118/

This LGTM

Eric Biagiotti wrote:

Peter Amstutz wrote:

[...]

Reverted back to parse_datetime_as_naive in 2e4ec565fc74e6de19fefd2a8670fd319544ce73

Most of our python code uses datetime.utcnow(), which is time zone naive. In order to be compatible with the removal of as_naive in ffef38539dd36e224abb75f0480cff1deb319124 we would need to update all these instances and introduce the pytz dependency for all versions of python. This seems unnecessary considering the original scope of this story and the fact that we don't currently have other problems related to datetimes.

Unit Tests: https://ci.curoverse.com/view/Developer/job/developer-run-tests/1099/
C&I Tests: https://ci.curoverse.com/view/CWL/job/arvados-cwl-conformance-tests/52/

Rerun of fuse tests: https://ci.curoverse.com/job/developer-run-tests-services-fuse/1118/

LGTM