EandrewJones commented on code in PR #53: URL: https://github.com/apache/flagon-distill/pull/53#discussion_r1679695046
########## distill/core/log.py: ########## @@ -57,4 +58,37 @@ def to_json(self) -> str: def to_dict(self) -> JsonDict: return self.data.model_dump(by_alias=True) - + + def __lt__(self, other): + if isinstance(other, Log): + return self.id < other.id + if isinstance(other, datetime): + return self.id.get_datetime() < other.now() Review Comment: This assumes `get_datetime()` returns an actual datetime object, but I don't think it does. Tracking down the method starting from the [PKSUID repo](https://github.com/sashahilton00/python-pksuid/blob/master/pksuid/pksuid.py) to the datetime [library docs](https://docs.python.org/3/library/datetime.html#datetime.date.fromtimestamp) suggests it just returns a float. Indeed, testing it in the terminal shows it just returns POSIX time float. I believe datetime objects have a `timestamp()` method that will return what we want. But we need to be careful to ensure get_datetime() and `other.timestamp()` are operating on the same scale. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@flagon.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@flagon.apache.org For additional commands, e-mail: notifications-h...@flagon.apache.org