Improved datetime comparison logic

[davidmrdavid]

Addresses: #168

This PR improves our datetime comparison logic, which is crucial for finding durable-timer-creation events in the state array. Before, we compared datetime objects using their toString representation, but some datetime subtypes (timezone aware datetimes) have a slightly different string representations of dates. As a result, some equivalent dates failed to actually be acknowledged as equal.

The solution here takes two steps. First, instead of comparing strings, we compare proper datetime objects against each other. However, timezone-aware datetimes and timezone-naive datetimes are not directly comparable. As a result, I force all datetime objects to be timezone-naive, but I'm unsure how safe this is, and worry about this breaking in the cloud.

I'm not sure of why we have the two datetime variants floating in our codebase. It would be good to unify their representations. Are the dates flowing through durable-extension always in the same timezone? If so, I could force them all to be under that timezone, which strikes me as safer than removing the timezone metadata.

Happy to explain this further, if needed!

[ConnorMcMahon]

In general, you can rely on the fact that the datetimes are UTC, so I believe stripping timezone data is OK... but I am not positive about that.

[ConnorMcMahon]

So this is the new datetime format we are using. Is there anyway for us to remove the reference to the old datetime format so it isn't accidentally used?

[davidmrdavid]

This is actually the same datetime format as before, I just needed to explicitly import it now, because I needed to use its datettime.strptime parser.

I'll write some more thoughts on the "two datetime variants" in a comment below.

[davidmrdavid]

I want to correct my previous explanation of the problem we were seeing, as it's no longer correct.

Problem Description

So here's concretely the issue. Given a CreateTimerAction object with a timestamp of "2020-07-23T21:56:54.936700Z", sometimes I would later receive a HistoryEvent object referring to that action with a slightly different timestamp: "2020-07-23T21:56:54.9367Z".

Note that the two timestamps are equivalent, but the latter drops redundant zeroes to the left. I think this is not happening at the Durable Python SDK, so I have to assume it's happening either at the durable-extension or DTFx (where do you think this is happening?). In any case, this means that a string-based timestamp comparison between the two dates will fail. The solution in this PR is to cast them back as datetime objects, where their representation will be equivalent.

I have also added a test to this PR to prevent regressions on this specific error.

On the 2 datetime representations

Finally, I also made this comment about having timezone-aware and timezone-naive datetime objects. I should not have made that comment now that I think about it because it explains nothing about the error. However, for completeness, the problem is that datetime objects created via context.current_utc_datetime will be timezone-aware and so they're not directly comparable with datetime ojects created via datettime.strptime(<timestamp str>). All that I need to do is to either make the latter object timezone aware (into UTC) or to remove the timezone awareness of the former. Anyways, that was just an implementation detail.

One final To-Do

The Python Durable tests also assert the correctness of the orchestration state schema and I'm getting errors when validating the correctness of the schema when timers are present. In particular, I'm seeing:

# "Additional properties are not allowed ('fireAt', 'isCanceled' were unexpected)">

Where can I find an up-to-date schema definition with these two fields? 🦓

[ConnorMcMahon]

LGTM!