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

Reply via email to