)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"545711f4d71d5cdeb18851d979057f0f9d0ac6ae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"0c4b7efb_e3768659","updated":"2026-05-06 14:06:05.000000000","message":"Andressa, Tim, thanks for the reviews. I have made some changes in response.","commit_id":"21a5cb4d12190cd2607f0ba1c8e07bc675fe421e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"765f2e80aa69115bc4ca7df930a389ad5800aec7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"231f6f9f_f2113e2d","updated":"2026-04-28 22:21:40.000000000","message":"I\u0027d be content to see this merged, but the more I poke at it, the more confused I get. Is there a reason for wanting to support floats? I\u0027m starting to come around toward some much more strict type checking, like `isinstance(value, int) and not isinstance(value, bool)`","commit_id":"21a5cb4d12190cd2607f0ba1c8e07bc675fe421e"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"90631d93fcdc368a2747c5ce873d1fd2d141018c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"aafc15ff_081c653f","updated":"2026-04-27 14:04:23.000000000","message":"Thanks for adding this check to make our code more reliable. I also liked that you added an unit test for the change. As I added in my comment, I\u0027m good with the current state of things.","commit_id":"21a5cb4d12190cd2607f0ba1c8e07bc675fe421e"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"5aa7e00a4b6f730c88dde7eb775d6c3af5f39309","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"4633391e_39a6f3b1","updated":"2026-05-07 11:05:00.000000000","message":"Thank you for kindly addressing my comment, this lgtm!","commit_id":"864f81bdf0a39aa4e75eb2a61bfd2bcd25664556"}],"swift/common/utils/timestamp.py":[{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"90631d93fcdc368a2747c5ce873d1fd2d141018c","unresolved":true,"context_lines":[{"line_number":103,"context_line":"            float_timestamp \u003d float(timestamp)"},{"line_number":104,"context_line":"            self.offset \u003d getattr(timestamp, \u0027offset\u0027, 0)"},{"line_number":105,"context_line":"        # increment offset"},{"line_number":106,"context_line":"        if int(offset) !\u003d offset:"},{"line_number":107,"context_line":"            raise ValueError(\u0027offset must be an integer value\u0027)"},{"line_number":108,"context_line":"        if offset \u003e\u003d 0:"},{"line_number":109,"context_line":"            self.offset +\u003d int(offset)"}],"source_content_type":"text/x-python","patch_set":1,"id":"b05b5a70_b223917c","line":106,"updated":"2026-04-27 14:04:23.000000000","message":"I\u0027m not sure if it\u0027s of our interest or not but I\u0027ll add this comment here in case we want to add an additional validation. Since bool is a subclass of integer the validation `int(offset) !\u003d offset` doesn\u0027t catch boolean values so Timestamp(...., offset\u003dTrue) won\u0027t be catch because Python will evaluate True to 1. Just to be totally clear, I\u0027m ok with the state of things, if you believe it\u0027s not necessary it\u0027s fine by me.","commit_id":"3c809dc60dff122eb99d51b9e3d37ff770d33810"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"765f2e80aa69115bc4ca7df930a389ad5800aec7","unresolved":true,"context_lines":[{"line_number":103,"context_line":"            float_timestamp \u003d float(timestamp)"},{"line_number":104,"context_line":"            self.offset \u003d getattr(timestamp, \u0027offset\u0027, 0)"},{"line_number":105,"context_line":"        # increment offset"},{"line_number":106,"context_line":"        if int(offset) !\u003d offset:"},{"line_number":107,"context_line":"            raise ValueError(\u0027offset must be an integer value\u0027)"},{"line_number":108,"context_line":"        if offset \u003e\u003d 0:"},{"line_number":109,"context_line":"            self.offset +\u003d int(offset)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ea897cfa_35565455","line":106,"in_reply_to":"b05b5a70_b223917c","updated":"2026-04-28 22:21:40.000000000","message":"A good reminder! Every now and then I get caught out by that, though I suppose it\u0027s nice that we can do things like\n```\n\u003e\u003e\u003e sum(\u0027a\u0027 in x for x in (\u0027foo\u0027, \u0027bar\u0027, \u0027baz\u0027))\n2\n```\n(Though I tend to prefer something more explicit like\n```\n\u003e\u003e\u003e sum(1 if \u0027a\u0027 in x else 0 for x in (\u0027foo\u0027, \u0027bar\u0027, \u0027baz\u0027))\n2\n```\nmyself.)\n\nI think the goal is more just to ensure we don\u0027t allow floats, though, since\n1. they previously caused things to blow up in unexpected ways and\n2. if we allowed them naively, we could lose the precision that callers supplied.","commit_id":"3c809dc60dff122eb99d51b9e3d37ff770d33810"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"545711f4d71d5cdeb18851d979057f0f9d0ac6ae","unresolved":true,"context_lines":[{"line_number":103,"context_line":"            float_timestamp \u003d float(timestamp)"},{"line_number":104,"context_line":"            self.offset \u003d getattr(timestamp, \u0027offset\u0027, 0)"},{"line_number":105,"context_line":"        # increment offset"},{"line_number":106,"context_line":"        if int(offset) !\u003d offset:"},{"line_number":107,"context_line":"            raise ValueError(\u0027offset must be an integer value\u0027)"},{"line_number":108,"context_line":"        if offset \u003e\u003d 0:"},{"line_number":109,"context_line":"            self.offset +\u003d int(offset)"}],"source_content_type":"text/x-python","patch_set":1,"id":"93d20153_d18aa220","line":106,"in_reply_to":"ea897cfa_35565455","updated":"2026-05-06 14:06:05.000000000","message":"A long time ago I \"helpfully\" tightened up a method that wasn\u0027t broken but was documented to accept integers. I added checks so that it really did only accept integers, rather than just casting to ``int``. Turned out that someone had been happily passing floats in and the change broke them, and they weren\u0027t happy. Since then I have been wary about unnecessarily making interfaces more brittle. So , since you could write ``Timestamp(123, offset\u003dTrue)`` before, I am inclined to leave it that way, however weird it looks.\n\nBut, kudos for the thorough review 👏","commit_id":"3c809dc60dff122eb99d51b9e3d37ff770d33810"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"765f2e80aa69115bc4ca7df930a389ad5800aec7","unresolved":true,"context_lines":[{"line_number":431,"context_line":"            return"},{"line_number":432,"context_line":"        if value \u003c 0:"},{"line_number":433,"context_line":"            raise ValueError(\u0027offset must be non-negative\u0027)"},{"line_number":434,"context_line":"        self.offset +\u003d value"},{"line_number":435,"context_line":"        return self.offset"},{"line_number":436,"context_line":""},{"line_number":437,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":3,"id":"4a260780_1bb9e268","line":434,"updated":"2026-04-28 22:21:40.000000000","message":"Now *this part* starts to feel a little sneaky/subtle -- we should maybe include a comment here about why we *don\u0027t* want/need an `int(...)` here. (Or maybe we do? IDK any more...)","commit_id":"21a5cb4d12190cd2607f0ba1c8e07bc675fe421e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"545711f4d71d5cdeb18851d979057f0f9d0ac6ae","unresolved":true,"context_lines":[{"line_number":431,"context_line":"            return"},{"line_number":432,"context_line":"        if value \u003c 0:"},{"line_number":433,"context_line":"            raise ValueError(\u0027offset must be non-negative\u0027)"},{"line_number":434,"context_line":"        self.offset +\u003d value"},{"line_number":435,"context_line":"        return self.offset"},{"line_number":436,"context_line":""},{"line_number":437,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":3,"id":"00058119_8ba5f82d","line":434,"in_reply_to":"4a260780_1bb9e268","updated":"2026-05-06 14:06:05.000000000","message":"I missed the potential TypeError that arises from not casting to int here, so I think that and the other validation does need to happen before the ``+\u003d``","commit_id":"21a5cb4d12190cd2607f0ba1c8e07bc675fe421e"}],"test/unit/common/utils/test_timestamp.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"765f2e80aa69115bc4ca7df930a389ad5800aec7","unresolved":true,"context_lines":[{"line_number":210,"context_line":"        # sanity check..."},{"line_number":211,"context_line":"        ts \u003d timestamp.Timestamp(1417462430.78693, offset\u003d1)"},{"line_number":212,"context_line":"        self.assertEqual(\u00271417462430.78693_0000000000000001\u0027, ts.internal)"},{"line_number":213,"context_line":"        # it\u0027s the value that matters, not the type..."},{"line_number":214,"context_line":"        ts \u003d timestamp.Timestamp(1417462430.78693, offset\u003d1.0)"},{"line_number":215,"context_line":"        self.assertEqual(\u00271417462430.78693_0000000000000001\u0027, ts.internal)"},{"line_number":216,"context_line":"        # only integer values are allowed..."}],"source_content_type":"text/x-python","patch_set":3,"id":"d792c85b_54c03479","line":213,"updated":"2026-04-28 22:21:40.000000000","message":"Huh. So... it\u0027s *intentional* that `t \u003d Timestamp.now(); t.offset\u003dt` will *sometimes* work? But `Timestamp.now(offset\u003dTimestamp.now())` will *always* raise a `TypeError`...","commit_id":"21a5cb4d12190cd2607f0ba1c8e07bc675fe421e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"545711f4d71d5cdeb18851d979057f0f9d0ac6ae","unresolved":true,"context_lines":[{"line_number":210,"context_line":"        # sanity check..."},{"line_number":211,"context_line":"        ts \u003d timestamp.Timestamp(1417462430.78693, offset\u003d1)"},{"line_number":212,"context_line":"        self.assertEqual(\u00271417462430.78693_0000000000000001\u0027, ts.internal)"},{"line_number":213,"context_line":"        # it\u0027s the value that matters, not the type..."},{"line_number":214,"context_line":"        ts \u003d timestamp.Timestamp(1417462430.78693, offset\u003d1.0)"},{"line_number":215,"context_line":"        self.assertEqual(\u00271417462430.78693_0000000000000001\u0027, ts.internal)"},{"line_number":216,"context_line":"        # only integer values are allowed..."}],"source_content_type":"text/x-python","patch_set":3,"id":"0b801aa4_927afc24","line":213,"in_reply_to":"d792c85b_54c03479","updated":"2026-05-06 14:06:05.000000000","message":"Fixed","commit_id":"21a5cb4d12190cd2607f0ba1c8e07bc675fe421e"}]}