)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da736e5acdec5cf64cea45deffea2d74c19a81ac","unresolved":true,"context_lines":[{"line_number":7,"context_line":"sq timestamps: simplify offset property"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For simplicity, get rid of _offset in favour of offset. As a result,"},{"line_number":10,"context_line":"restore the ability to set offst to the public interface."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ibedc4a8191c4778a6b100f1e91ba6998df2e0f28"},{"line_number":13,"context_line":"Signed-off-by: Alistair Coles \u003calistairncoles@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"93994fd2_fa9885cf","line":10,"updated":"2026-05-14 15:56:54.000000000","message":"\u003e restore the ability to set offst to the public interface\n\nI don\u0027t know that we HAVE to do this - it seemed like we were getting some buy-in that offset SHOULD have been read-only and ONLY *incremented* via `increment_offset` (despite new tests starting to verify it\u0027s not a dangerous foot-gun if you DO set it - those tests could just be updated to set `_offset`)","commit_id":"eae8b6b7a6b2c04cd857783262b1bc584bac8c62"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d21826ebb7ed0d63215121a583b5e6103c4745b8","unresolved":false,"context_lines":[{"line_number":7,"context_line":"sq timestamps: simplify offset property"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For simplicity, get rid of _offset in favour of offset. As a result,"},{"line_number":10,"context_line":"restore the ability to set offst to the public interface."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ibedc4a8191c4778a6b100f1e91ba6998df2e0f28"},{"line_number":13,"context_line":"Signed-off-by: Alistair Coles \u003calistairncoles@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"43fcb739_c35c4732","line":10,"in_reply_to":"93994fd2_fa9885cf","updated":"2026-05-15 10:38:48.000000000","message":"Acknowledged","commit_id":"eae8b6b7a6b2c04cd857783262b1bc584bac8c62"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da736e5acdec5cf64cea45deffea2d74c19a81ac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e633b002_7dcae898","updated":"2026-05-14 15:56:54.000000000","message":"took me a minute to come around on \"unknown version makes offset AttributeError\" - but I think it\u0027s actually the right call.\n\nI don\u0027t want my positive vote in ANY WAY to be read as advocating \"correct; we need to keep offset setter as public\" - I actually think that a) tests that setting offset don\u0027t ruin things in unexpected ways was good but also b) if we make offset a read-only property b/c we don\u0027t want people to try to set offset directly - that would ALSO be a good thing\n\nand even if we do \"b\" (I think we should; maybe on master) I think it just moves the surface area of `self._offset \u003d -1.0` to a new layer - which should *also* probably NOT \"ruin things in unexpected ways\" (so the investment in \"a\" is still value even if we continue to refine and improve and strengthen the actual interface)","commit_id":"eae8b6b7a6b2c04cd857783262b1bc584bac8c62"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d21826ebb7ed0d63215121a583b5e6103c4745b8","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c239c1ad_81595a68","in_reply_to":"e633b002_7dcae898","updated":"2026-05-15 10:38:48.000000000","message":"\u003e I don\u0027t want my positive vote in ANY WAY to be read as advocating \"correct; we need to keep offset setter as public\" \n👍\nit\u0027s about not making the change to offset be a drive-by in v2 timestamps patch. We can make offset private independently of this (parent) patch.","commit_id":"eae8b6b7a6b2c04cd857783262b1bc584bac8c62"}],"swift/common/utils/timestamp.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da736e5acdec5cf64cea45deffea2d74c19a81ac","unresolved":true,"context_lines":[{"line_number":477,"context_line":"    def offset(self):"},{"line_number":478,"context_line":"        \"\"\""},{"line_number":479,"context_line":"        This is part of the public interface for backwards compatibility only."},{"line_number":480,"context_line":"        Callers should not attach any meaning to the absolute value of offset."},{"line_number":481,"context_line":"        \"\"\""},{"line_number":482,"context_line":"        if self.version \u003d\u003d 1:"},{"line_number":483,"context_line":"            # Previous versions of this class allowed offset to grow beyond the"}],"source_content_type":"text/x-python","patch_set":1,"id":"43e779fc_9e898f18","line":480,"updated":"2026-05-14 15:56:54.000000000","message":"\u003e Callers should not attach any meaning to the absolute value of offset\n\nI\u0027m not sure I understand exactly what this means... \n\nthe \"meaning\" of offset has to do with ordering \"timestamps\" of equal immutable value.\n\nYou could say: \u0027well yeah, but you don\u0027t REALLY care about the magnitude!\" and that\u0027s probably mostly true except callers do care about manipulating the order of increments for puts and deletes to the point where `increment_offset(1)` and `increment_offset(3)` are both supported/used/useful\n\nthat suggests to me something about the \"value\" is relevant","commit_id":"eae8b6b7a6b2c04cd857783262b1bc584bac8c62"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d21826ebb7ed0d63215121a583b5e6103c4745b8","unresolved":false,"context_lines":[{"line_number":477,"context_line":"    def offset(self):"},{"line_number":478,"context_line":"        \"\"\""},{"line_number":479,"context_line":"        This is part of the public interface for backwards compatibility only."},{"line_number":480,"context_line":"        Callers should not attach any meaning to the absolute value of offset."},{"line_number":481,"context_line":"        \"\"\""},{"line_number":482,"context_line":"        if self.version \u003d\u003d 1:"},{"line_number":483,"context_line":"            # Previous versions of this class allowed offset to grow beyond the"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f75fa42_07dd7989","line":480,"in_reply_to":"43e779fc_9e898f18","updated":"2026-05-15 10:38:48.000000000","message":"I\u0027m going to revert these docstring changes so that the v2 timestamp patch doesn\u0027t start to make implications about this interface","commit_id":"eae8b6b7a6b2c04cd857783262b1bc584bac8c62"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da736e5acdec5cf64cea45deffea2d74c19a81ac","unresolved":true,"context_lines":[{"line_number":497,"context_line":"    @offset.setter"},{"line_number":498,"context_line":"    def offset(self, value):"},{"line_number":499,"context_line":"        \"\"\""},{"line_number":500,"context_line":"        This is part of the public interface for backwards compatibility only."},{"line_number":501,"context_line":"        Callers should typically use ``increment_offset`` to modify the offset"},{"line_number":502,"context_line":"        value."},{"line_number":503,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9bcc5db1_979596d3","line":500,"updated":"2026-05-14 15:56:54.000000000","message":"do we want to keep this, can we change this?\n\n```\n(vagrant-swift-all-in-one) cgerrard@NVStation:~/Workspace/scratch/property$ cat private_property.py \nclass MyObject(object):\n\n    def __init__(self):\n        self._hexpart \u003d 0x200000\n\n    @property\n    def offset(self):\n        return self._hexpart \u0026 0x00ffff\n\n    _offset \u003d offset\n\n    @_offset.setter\n    def _offset(self, value):\n        self._hexpart \u003d self._hexpart + value\n\n\nimport pdb\npdb.set_trace()\n```\n\nI think we could make the setter private?","commit_id":"eae8b6b7a6b2c04cd857783262b1bc584bac8c62"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d21826ebb7ed0d63215121a583b5e6103c4745b8","unresolved":false,"context_lines":[{"line_number":497,"context_line":"    @offset.setter"},{"line_number":498,"context_line":"    def offset(self, value):"},{"line_number":499,"context_line":"        \"\"\""},{"line_number":500,"context_line":"        This is part of the public interface for backwards compatibility only."},{"line_number":501,"context_line":"        Callers should typically use ``increment_offset`` to modify the offset"},{"line_number":502,"context_line":"        value."},{"line_number":503,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"13271c0a_4e7112b8","line":500,"in_reply_to":"9bcc5db1_979596d3","updated":"2026-05-15 10:38:48.000000000","message":"Agree we could, I\u0027m inclined to tackle that outside of the v2 timestamps patch, so this squash is just making it that the v2 timestamps patch doesn\u0027t remove the public ``offset`` attribute as a drive-by.","commit_id":"eae8b6b7a6b2c04cd857783262b1bc584bac8c62"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da736e5acdec5cf64cea45deffea2d74c19a81ac","unresolved":true,"context_lines":[{"line_number":499,"context_line":"        \"\"\""},{"line_number":500,"context_line":"        This is part of the public interface for backwards compatibility only."},{"line_number":501,"context_line":"        Callers should typically use ``increment_offset`` to modify the offset"},{"line_number":502,"context_line":"        value."},{"line_number":503,"context_line":""},{"line_number":504,"context_line":"        :param value: (int) increment to the second internal offset vector."},{"line_number":505,"context_line":"        :raises ValueError: if value is not a whole number or exceeds the"}],"source_content_type":"text/x-python","patch_set":1,"id":"8a7b446f_bbbc3e43","line":502,"updated":"2026-05-14 15:56:54.000000000","message":"oic oic oic - yeah it\u0027s a little tricky - mostly we probably *did* ignore `offset` the attribute until we started adding a bunch of tests for it (proved by virtue of how busted it could be to use it incorrectly)","commit_id":"eae8b6b7a6b2c04cd857783262b1bc584bac8c62"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da736e5acdec5cf64cea45deffea2d74c19a81ac","unresolved":true,"context_lines":[{"line_number":519,"context_line":"        else:"},{"line_number":520,"context_line":"            # we don\u0027t know how many hex digits have been allocated to offset"},{"line_number":521,"context_line":"            # in a future version of this class so it is not safe to modify it"},{"line_number":522,"context_line":"            raise AttributeError(\u0027Cannot access offset in an unrecognised \u0027"},{"line_number":523,"context_line":"                                 \u0027hex_part encoding\u0027)"},{"line_number":524,"context_line":"        if value \u003e max_offset:"},{"line_number":525,"context_line":"            raise ValueError(\u0027offset must be less than or equal to %d\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"b105c6e3_48dca240","line":522,"updated":"2026-05-14 15:56:54.000000000","message":"I wonder if this is the \"right\" error type\n\nValueError vs AttributeError seem quite orthogonal:\n\n```\n\u003e\u003e\u003e isinstance(AttributeError, ValueError)\nFalse\n\u003e\u003e\u003e AttributeError.__mro__\n(\u003cclass \u0027AttributeError\u0027\u003e, \u003cclass \u0027Exception\u0027\u003e, \u003cclass \u0027BaseException\u0027\u003e, \u003cclass \u0027object\u0027\u003e)\n\u003e\u003e\u003e ValueError.__mro__\n(\u003cclass \u0027ValueError\u0027\u003e, \u003cclass \u0027Exception\u0027\u003e, \u003cclass \u0027BaseException\u0027\u003e, \u003cclass \u0027object\u0027\u003e)\n```\n\nMaybe the justification is something like \"offset becomes read-only for un-recognized version\" - AttributeError might make sense in that case:\n\n```\n(Pdb) o.offset \u003d \u0027foo\u0027\n*** AttributeError: property \u0027offset\u0027 of \u0027MyObject\u0027 object has no setter\n``\n\n... except I don\u0027t think we let you READ offset either\n\nI\u0027m pretty sure the ValueError was fine; but I get that it won\u0027t really matter what *value* the users passes in - we can\u0027t set offset for unknown version; so it\u0027s \"like\" the offset \"property\" doesn\u0027t exist.","commit_id":"eae8b6b7a6b2c04cd857783262b1bc584bac8c62"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d21826ebb7ed0d63215121a583b5e6103c4745b8","unresolved":false,"context_lines":[{"line_number":519,"context_line":"        else:"},{"line_number":520,"context_line":"            # we don\u0027t know how many hex digits have been allocated to offset"},{"line_number":521,"context_line":"            # in a future version of this class so it is not safe to modify it"},{"line_number":522,"context_line":"            raise AttributeError(\u0027Cannot access offset in an unrecognised \u0027"},{"line_number":523,"context_line":"                                 \u0027hex_part encoding\u0027)"},{"line_number":524,"context_line":"        if value \u003e max_offset:"},{"line_number":525,"context_line":"            raise ValueError(\u0027offset must be less than or equal to %d\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"21f69103_686f7718","line":522,"in_reply_to":"b105c6e3_48dca240","updated":"2026-05-15 10:38:48.000000000","message":"Done","commit_id":"eae8b6b7a6b2c04cd857783262b1bc584bac8c62"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da736e5acdec5cf64cea45deffea2d74c19a81ac","unresolved":true,"context_lines":[{"line_number":534,"context_line":"        :param value: (int) increment to the second internal offset vector."},{"line_number":535,"context_line":"        :raises ValueError: if value is not a whole number, or if the"},{"line_number":536,"context_line":"            resulting offset would exceed the maximum supported offset."},{"line_number":537,"context_line":"        :raises AttributeError: if the Timestamp version is unknown."},{"line_number":538,"context_line":"        \"\"\""},{"line_number":539,"context_line":"        if not value:"},{"line_number":540,"context_line":"            return self.offset"}],"source_content_type":"text/x-python","patch_set":1,"id":"d4dc9669_0cdb983c","line":537,"updated":"2026-05-14 15:56:54.000000000","message":"lol, rly?","commit_id":"eae8b6b7a6b2c04cd857783262b1bc584bac8c62"}]}