octue / octue-sdk-python

The python SDK for @Octue services and digital twins.
https://octue.com
Other
10 stars 4 forks source link

Order events by datetime instead of `order` #663

Closed cortadocodes closed 4 months ago

cortadocodes commented 5 months ago

Bug report

What is the current behavior?

When getting events, both the question and the first response event from the child have order zero, so we get a duplicate event warning and can only see one of those events.

What is the expected behavior?

We should be able to get the question and the first response with no warning.

Proposed Solution

Order events by datetime in the event handlers instead of the order attribute.

cortadocodes commented 4 months ago

The only downside I can think of for this is that we lose the ability to know if:

I think it's worth it though as it will:

  1. Simplify the event handler quite a bit
  2. While still providing a good certainty in the order of the events (we control the datetime set in the events)
  3. Allow us to handle any group of events we like, even if they weren't emitted from the same source (e.g. the case of the question event plus the set of response events)

I think carrying on using the order attribute is actually tech-debt now that we've moved towards an event driven architecture.

thclark commented 4 months ago

I think on the basis that we don't ever, in practicality, actually do anything if we're missing messages, this is a small price to pay for a great simplification in an area where actually we've accumulated a lot of unnecessary complexity. So I'm all for this.

cortadocodes commented 4 months ago

I've realised that, for synchronous questions, "ordering by datetime" would in fact mean just handling the events in the order that pub/sub sends them in. This is because datetimes are continuous whereas the integer order attribute was discrete.

We'll try this without a pub/sub ordering key and add one if it becomes an issue. Asynchronous questions can still be ordered by datetime.