Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 98
model: fix timezone handling for Edm.DateTimeOffset#184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -29,17 +29,6 @@ | ||||
| TypeInfo = collections.namedtuple('TypeInfo', 'namespace name is_collection') | ||||
| def current_timezone(): | ||||
| """Default Timezone for Python datetime instances when parsed from | ||||
| Edm.DateTime values and vice versa. | ||||
| OData V2 does not mention Timezones in the documentation of | ||||
| Edm.DateTime and UTC was chosen because it is universal. | ||||
| """ | ||||
| return datetime.timezone.utc | ||||
| def modlog(): | ||||
| return logging.getLogger(LOGGER_NAME) | ||||
| @@ -210,7 +199,8 @@ def _build_types(): | ||||
| Types.register_type(Typ('Edm.SByte', '0')) | ||||
| Types.register_type(Typ('Edm.String', '\'\'', EdmStringTypTraits())) | ||||
| Types.register_type(Typ('Edm.Time', 'time\'PT00H00M\'')) | ||||
| Types.register_type(Typ('Edm.DateTimeOffset', 'datetimeoffset\'0000-00-00T00:00:00\'')) | ||||
| Types.register_type( | ||||
| Typ('Edm.DateTimeOffset', 'datetimeoffset\'0000-00-00T00:00:00Z\'', EdmDateTimeOffsetTypTraits())) | ||||
| @staticmethod | ||||
| def register_type(typ): | ||||
| @@ -373,6 +363,40 @@ def from_literal(self, value): | ||||
| return base64.b64encode(binary).decode() | ||||
| def ms_since_epoch_to_datetime(value, tzinfo): | ||||
rettichschnidi marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||||
| """Convert milliseconds since midnight 1.1.1970 to datetime""" | ||||
| try: | ||||
| # https://stackoverflow.com/questions/36179914/timestamp-out-of-range-for-platform-localtime-gmtime-function | ||||
| return datetime.datetime(1970, 1, 1, tzinfo=tzinfo) + datetime.timedelta(milliseconds=int(value)) | ||||
| except (ValueError, OverflowError): | ||||
| min_ticks = -62135596800000 | ||||
| max_ticks = 253402300799999 | ||||
| if FIX_SCREWED_UP_MINIMAL_DATETIME_VALUE and int(value) < min_ticks: | ||||
| # Some service providers return false minimal date values. | ||||
| # -62135596800000 is the lowest value PyOData could read. | ||||
| # This workaround fixes this issue and returns 0001-01-01 00:00:00+00:00 in such a case. | ||||
| return datetime.datetime(year=1, day=1, month=1, tzinfo=tzinfo) | ||||
| if FIX_SCREWED_UP_MAXIMUM_DATETIME_VALUE and int(value) > max_ticks: | ||||
| return datetime.datetime(year=9999, day=31, month=12, tzinfo=tzinfo) | ||||
| raise PyODataModelError(f'Cannot decode datetime from value {value}. ' | ||||
| f'Possible value range: {min_ticks} to {max_ticks}. ' | ||||
| f'You may fix this by setting `FIX_SCREWED_UP_MINIMAL_DATETIME_VALUE` ' | ||||
| f' or `FIX_SCREWED_UP_MAXIMUM_DATETIME_VALUE` as a workaround.') | ||||
| def parse_datetime_literal(value): | ||||
| ||||
| try: | ||||
| return datetime.datetime.strptime(value, '%Y-%m-%dT%H:%M:%S.%f') | ||||
| except ValueError: | ||||
| try: | ||||
| return datetime.datetime.strptime(value, '%Y-%m-%dT%H:%M:%S') | ||||
| except ValueError: | ||||
| try: | ||||
| return datetime.datetime.strptime(value, '%Y-%m-%dT%H:%M') | ||||
| except ValueError: | ||||
| raise PyODataModelError(f'Cannot decode datetime from value {value}.') | ||||
| class EdmDateTimeTypTraits(EdmPrefixedTypTraits): | ||||
| """Emd.DateTime traits | ||||
| @@ -403,46 +427,48 @@ def to_literal(self, value): | ||||
| raise PyODataModelError( | ||||
| f'Cannot convert value of type {type(value)} to literal. Datetime format is required.') | ||||
| if value.tzinfo != datetime.timezone.utc: | ||||
| raise PyODataModelError('Emd.DateTime accepts only UTC') | ||||
| # Sets timezone to none to avoid including timezone information in the literal form. | ||||
| return super(EdmDateTimeTypTraits, self).to_literal(value.replace(tzinfo=None).isoformat()) | ||||
| def to_json(self, value): | ||||
| if isinstance(value, str): | ||||
| return value | ||||
| if value.tzinfo != datetime.timezone.utc: | ||||
| raise PyODataModelError('Emd.DateTime accepts only UTC') | ||||
| # Converts datetime into timestamp in milliseconds in UTC timezone as defined in ODATA specification | ||||
| # https://www.odata.org/documentation/odata-version-2-0/json-format/ | ||||
| return f'/Date({int(value.replace(tzinfo=current_timezone()).timestamp()) * 1000})/' | ||||
| # See also: https://docs.python.org/3/library/datetime.html#datetime.datetime.timestamp | ||||
| ticks = (value - datetime.datetime(1970, 1, 1, tzinfo=datetime.timezone.utc)) / datetime.timedelta(milliseconds=1) | ||||
| return f'/Date({int(ticks)})/' | ||||
| def from_json(self, value): | ||||
| if value is None: | ||||
| return None | ||||
| matches = re.match(r"^/Date\((.*)\)/$", value) | ||||
| if not matches: | ||||
| matches = re.match(r"^/Date\((?P<milliseconds_since_epoch>-?\d+)(?P<offset_in_minutes>[+-]\d+)?\)/$", value) | ||||
| try: | ||||
| milliseconds_since_epoch = matches.group('milliseconds_since_epoch') | ||||
| except AttributeError: | ||||
| raise PyODataModelError( | ||||
| f"Malformed value {value} for primitive Edm type. Expected format is /Date(value)/") | ||||
| value = matches.group(1) | ||||
| f"Malformed value {value} for primitive Edm.DateTime type." | ||||
| " Expected format is /Date(<ticks>[±<offset>])/") | ||||
| try: | ||||
| # https://stackoverflow.com/questions/36179914/timestamp-out-of-range-for-platform-localtime-gmtime-function | ||||
| value = datetime.datetime(1970, 1, 1, tzinfo=current_timezone()) + datetime.timedelta(milliseconds=int(value)) | ||||
| except (ValueError, OverflowError): | ||||
| if FIX_SCREWED_UP_MINIMAL_DATETIME_VALUE and int(value) < -62135596800000: | ||||
| # Some service providers return false minimal date values. | ||||
| # -62135596800000 is the lowest value PyOData could read. | ||||
| # This workaroud fixes this issue and returns 0001-01-01 00:00:00+00:00 in such a case. | ||||
| value = datetime.datetime(year=1, day=1, month=1, tzinfo=current_timezone()) | ||||
| elif FIX_SCREWED_UP_MAXIMUM_DATETIME_VALUE and int(value) > 253402300799999: | ||||
| value = datetime.datetime(year=9999, day=31, month=12, tzinfo=current_timezone()) | ||||
| else: | ||||
| raise PyODataModelError(f'Cannot decode datetime from value {value}. ' | ||||
| f'Possible value range: -62135596800000 to 253402300799999. ' | ||||
| f'You may fix this by setting `FIX_SCREWED_UP_MINIMAL_DATETIME_VALUE` ' | ||||
| f' or `FIX_SCREWED_UP_MAXIMUM_DATETIME_VALUE` as a workaround.') | ||||
| return value | ||||
| offset_in_minutes = int(matches.group('offset_in_minutes') or 0) | ||||
| timedelta = datetime.timedelta(minutes=offset_in_minutes) | ||||
| except ValueError: | ||||
| raise PyODataModelError( | ||||
| f"Malformed value {value} for primitive Edm.DateTime type." | ||||
| " Expected format is /Date(<ticks>[±<offset>])/") | ||||
| except AttributeError: | ||||
| timedelta = datetime.timedelta() # Missing offset is interpreted as UTC | ||||
| # Might raise a PyODataModelError exception | ||||
| return ms_since_epoch_to_datetime(milliseconds_since_epoch, datetime.timezone.utc) + timedelta | ||||
| def from_literal(self, value): | ||||
| @@ -451,18 +477,85 @@ def from_literal(self, value): | ||||
| value = super(EdmDateTimeTypTraits, self).from_literal(value) | ||||
| # Note: parse_datetime_literal raises a PyODataModelError exception on invalid formats | ||||
| return parse_datetime_literal(value).replace(tzinfo=datetime.timezone.utc) | ||||
| class EdmDateTimeOffsetTypTraits(EdmPrefixedTypTraits): | ||||
| """Emd.DateTimeOffset traits | ||||
| Represents date and time, plus an offset in minutes from UTC, with values ranging from 12:00:00 midnight, | ||||
| January 1, 1753 A.D. through 11:59:59 P.M, December 9999 A.D | ||||
| Literal forms: | ||||
| datetimeoffset'yyyy-mm-ddThh:mm[:ss]±ii:nn' (works for all time zones) | ||||
| datetimeoffset'yyyy-mm-ddThh:mm[:ss]Z' (works only for UTC) | ||||
| NOTE: Spaces are not allowed between datetimeoffset and quoted portion. | ||||
| The datetime part is case-insensitive, the offset one is not. | ||||
| Example 1: datetimeoffset'1970-01-01T00:00:01+00:30' | ||||
| - /Date(1000+0030)/ (As DateTime, but with a 30 minutes timezone offset) | ||||
| Example 1: datetimeoffset'1970-01-01T00:00:01-00:60' | ||||
| - /Date(1000-0030)/ (As DateTime, but with a negative 60 minutes timezone offset) | ||||
| https://blogs.sap.com/2017/01/05/date-and-time-in-sap-gateway-foundation/ | ||||
| ||||
| # TODO: create a class SAP attributes |
Did you find any difference in behavior of timezones that would be SAP specific - and not applicaple to "odata model in general" in model.py? AFAIK, all your changes so far should be odata-generic, without any need for SAP special handling (i.e. SAP adhers to DateTime and DateTimeOffset odata specification).
actual line comment relevant:
Everything previous is little tangential, I only commented that I would use the link to odata specification (as scattered in comments in this PR) instead of SAP blog post, that's all.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will copypaste this... I am just asking.. regexp also the only option?
rettichschnidiJan 4, 2022 •
edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dismissed alternatives:
- fromisoformat - but this needs Python 3.7+. Ubuntu 18.04 will be EOL soon - any chance you will drop support 3.6?
- python-dateutil - would be a new dependency
phanak-sapJan 4, 2022 •
edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for explanation of the thoughts behind this line.
Actually, since you asked.. yes, I have intention in near future drop support for Python 3..6, since it is EOL, will not receive any security fixes - and pyodata is a type of library that most probably will be used in more fresh environments, simply since you want the latest requests library as well, I guess.
So far there was no actual need for code change that would be Py3.6 incompatible - so I wanted to mark 3.6 python build as possible to fail, but still monitor them. But 3.6 backward-compatibility really does not need to be enforced.
I would go in a way that we create an spinoff issue for the fromisoformat refactoring in the future (the more standard library the better IMHO, we need only LXML and request-like lib so far, so no new dependancy), that would depend on actual drop of Py3.6 support in pyodata - I created issue #190 for that. In this PR, let's stay on what you got already.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.