)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"d6fe52921a4f601219c805f07fb9e1bc2c2933c1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"541f9dbd_c0d0a8db","updated":"2025-03-31 00:10:13.000000000","message":"Looking good guys. Some comments inline. And also need to start thinking about a test here too.","commit_id":"a4a71ec0be7c12657163754bff5195b47b9df76c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"80eb5acb060e67afaf0886c4e62554bdefb92e14","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ab2eea50_a0ddc542","updated":"2025-05-16 23:24:30.000000000","message":"Assuming it doesn\u0027t involve _too much_ test churn, I think it\u0027d be good if we updated the other callers of `update_metadata` and `update_put_timestamp`, at least under `swift/`:\n```\n$ egrep -r \u0027\\.update_(metadata|put_timestamp)\\(\u0027 swift\nswift/cli/manage_shard_ranges.py:        broker.update_metadata({\u0027X-Container-Sysmeta-Sharding\u0027:\nswift/common/db.py:        self.update_metadata(cleared_meta)\nswift/common/db_replicator.py:                broker.update_metadata(json.loads(rinfo[\u0027metadata\u0027]))\nswift/common/db_replicator.py:                broker.update_metadata(remote_info[\u0027metadata\u0027])\nswift/common/db_replicator.py:        new_broker.update_metadata(existing_broker.metadata)\nswift/container/sharder.py:        shard_broker.update_metadata({\nswift/container/backend.py:        fresh_broker.update_metadata(self.metadata)\nswift/container/backend.py:        self.update_metadata({\u0027X-Container-Sysmeta-Shard-\u0027 + key:\nswift/container/server.py:        broker.update_put_timestamp(timestamp)\nswift/container/server.py:                broker.update_metadata(metadata, validate_metadata\u003dTrue)\nswift/account/server.py:            broker.update_metadata(metadata, validate_metadata\u003dTrue)\nswift/account/server.py:                broker.update_put_timestamp(timestamp.internal)\n```\n(There\u0027s a bunch more under `test/`, but that seems significant enough to distract from the thrust of this patch. Probably better as a follow-on patch.)","commit_id":"f41265a952fcac15a3e4985023bd80b8f7104809"}],"swift/common/db.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"d6fe52921a4f601219c805f07fb9e1bc2c2933c1","unresolved":true,"context_lines":[{"line_number":1087,"context_line":"                    md[key] \u003d value_timestamp"},{"line_number":1088,"context_line":"            if validate_metadata:"},{"line_number":1089,"context_line":"                DatabaseBroker.validate_metadata(md)"},{"line_number":1090,"context_line":"            conn.execute(\u0027UPDATE %s_stat SET metadata \u003d ?\u0027 % self.db_type,"},{"line_number":1091,"context_line":"                         (json.dumps(md),))"},{"line_number":1092,"context_line":"            conn.commit()"},{"line_number":1093,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9ccb4250_af4dcfa1","line":1090,"range":{"start_line":1090,"start_character":26,"end_line":1090,"end_character":58},"updated":"2025-03-31 00:10:13.000000000","message":"Seeing as we\u0027re updating the same table twice. Should we just do it once, to make sure it\u0027s atomic in this case:\n\n```\nUPDATE %s_stat SET put_timestamp \u003d ?, metadata \u003d ?\n```\n\nOh but we have the `WHERE put_timestamp \u003c ?` condition.. so maybe we can use CASE.. never used it before, but aparently we can use it.. maybe something like:\n\n```\nUPDATE %s_stat SET\n  put_timestamp \u003d CASE WHEN put_timestamp \u003c ? THEN ? ELSE put_timestamp END,\n  metadata \u003d ?\n```\nhttps://www.sqlitetutorial.net/sqlite-case/","commit_id":"a4a71ec0be7c12657163754bff5195b47b9df76c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a6114cb73a6c95ce633dfd33f1b22e00b383c22d","unresolved":true,"context_lines":[{"line_number":998,"context_line":"            raise HTTPBadRequest(\u0027Total metadata too large; max %d\u0027"},{"line_number":999,"context_line":"                                 % MAX_META_OVERALL_SIZE)"},{"line_number":1000,"context_line":""},{"line_number":1001,"context_line":"    def update_metadata(self, metadata_updates, validate_metadata\u003dFalse):"},{"line_number":1002,"context_line":"        \"\"\""},{"line_number":1003,"context_line":"        Updates the metadata dict for the database. The metadata dict values"},{"line_number":1004,"context_line":"        are tuples of (value, timestamp) where the timestamp indicates when"}],"source_content_type":"text/x-python","patch_set":3,"id":"25377ae2_df3ce558","line":1001,"updated":"2025-04-29 00:22:23.000000000","message":"I wonder if we could replace all calls into this with `update_metadata_and_put_timestamp` -- to the degree we might need to _only_ update metadata, we could allow the `req_timestamp` to be explicitly `None`.\n\n(We might still want to keep this function around, but simplify it down to\n```\ndef update_metadata(self, metadata_updates, validate_metadata\u003dFalse):\n    # log some deprecation warning\n    return self.update_metadata_and_put_timestamp(metadata_updates, None, validate_metadata)\n```\n)","commit_id":"bd8c9c0eab71ffd134e080cf7307e4d0cf29c415"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a6114cb73a6c95ce633dfd33f1b22e00b383c22d","unresolved":true,"context_lines":[{"line_number":1057,"context_line":"                if timestamp \u003e old_metadata[key][1]:"},{"line_number":1058,"context_line":"                    break"},{"line_number":1059,"context_line":"            else:"},{"line_number":1060,"context_line":"                return"},{"line_number":1061,"context_line":""},{"line_number":1062,"context_line":"        with self.get() as conn:"},{"line_number":1063,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3683b869_f75200da","line":1060,"range":{"start_line":1060,"start_character":16,"end_line":1060,"end_character":22},"updated":"2025-04-29 00:22:23.000000000","message":"We still want to update the timestamp, though, yeah? Looking ahead, I think we might still want this check (despite the similarity to L1079-1082), but only return early `if req_timestamp is None`","commit_id":"bd8c9c0eab71ffd134e080cf7307e4d0cf29c415"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a6114cb73a6c95ce633dfd33f1b22e00b383c22d","unresolved":true,"context_lines":[{"line_number":1081,"context_line":"                if key not in md or timestamp \u003e md[key][1]:"},{"line_number":1082,"context_line":"                    md[key] \u003d value_timestamp"},{"line_number":1083,"context_line":"            if validate_metadata:"},{"line_number":1084,"context_line":"                DatabaseBroker.validate_metadata(md)"},{"line_number":1085,"context_line":"            conn.execute("},{"line_number":1086,"context_line":"                \u0027\u0027\u0027UPDATE %s_stat SET"},{"line_number":1087,"context_line":"                       put_timestamp \u003d CASE"}],"source_content_type":"text/x-python","patch_set":3,"id":"9bde8d5f_3a26ded1","line":1084,"range":{"start_line":1084,"start_character":16,"end_line":1084,"end_character":30},"updated":"2025-04-29 00:22:23.000000000","message":"Not `self.validate_metadata(md)`? Oh, I see -- this is what `update_metadata` currently does, and we don\u0027t currently override/extend it, anyway...","commit_id":"bd8c9c0eab71ffd134e080cf7307e4d0cf29c415"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a6114cb73a6c95ce633dfd33f1b22e00b383c22d","unresolved":true,"context_lines":[{"line_number":1084,"context_line":"                DatabaseBroker.validate_metadata(md)"},{"line_number":1085,"context_line":"            conn.execute("},{"line_number":1086,"context_line":"                \u0027\u0027\u0027UPDATE %s_stat SET"},{"line_number":1087,"context_line":"                       put_timestamp \u003d CASE"},{"line_number":1088,"context_line":"                           WHEN put_timestamp \u003c ? THEN ?"},{"line_number":1089,"context_line":"                           ELSE put_timestamp"},{"line_number":1090,"context_line":"                       END,"},{"line_number":1091,"context_line":"                       metadata \u003d ?\u0027\u0027\u0027 % self.db_type,"},{"line_number":1092,"context_line":"                (req_timestamp.internal, req_timestamp.internal,"},{"line_number":1093,"context_line":"                 json.dumps(md),))"}],"source_content_type":"text/x-python","patch_set":3,"id":"7839e819_0ea400c3","line":1090,"range":{"start_line":1087,"start_character":39,"end_line":1090,"end_character":26},"updated":"2025-04-29 00:22:23.000000000","message":"`MAX(put_stimestamp, ?)` maybe?","commit_id":"bd8c9c0eab71ffd134e080cf7307e4d0cf29c415"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a6114cb73a6c95ce633dfd33f1b22e00b383c22d","unresolved":true,"context_lines":[{"line_number":1180,"context_line":"                raise"},{"line_number":1181,"context_line":"        return False"},{"line_number":1182,"context_line":""},{"line_number":1183,"context_line":"    def update_put_timestamp(self, timestamp):"},{"line_number":1184,"context_line":"        \"\"\""},{"line_number":1185,"context_line":"        Update the put_timestamp.  Only modifies it if it is greater than"},{"line_number":1186,"context_line":"        the current timestamp."}],"source_content_type":"text/x-python","patch_set":3,"id":"941eb510_5f140d16","line":1183,"updated":"2025-04-29 00:22:23.000000000","message":"Here, too -- we might could update callers to use `update_metadata_and_put_timestamp` instead, but leave a stub like\n```\ndef update_put_timestamp(self, timestamp):\n    # log some deprecation warning\n    return self.update_metadata_and_put_timestamp({}, timestamp, False)\n```","commit_id":"bd8c9c0eab71ffd134e080cf7307e4d0cf29c415"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"80eb5acb060e67afaf0886c4e62554bdefb92e14","unresolved":true,"context_lines":[{"line_number":1000,"context_line":""},{"line_number":1001,"context_line":"    def update_metadata(self, metadata_updates, validate_metadata\u003dFalse):"},{"line_number":1002,"context_line":"        \"\"\""},{"line_number":1003,"context_line":"        Updates the metadata dict for the database. The metadata dict values"},{"line_number":1004,"context_line":"        are tuples of (value, timestamp) where the timestamp indicates when"},{"line_number":1005,"context_line":"        that key was set to that value. Key/values will only be overwritten if"},{"line_number":1006,"context_line":"        the timestamp is newer. To delete a key, set its value to (\u0027\u0027,"},{"line_number":1007,"context_line":"        timestamp). These empty keys will eventually be removed by"},{"line_number":1008,"context_line":"        :func:`reclaim`"},{"line_number":1009,"context_line":"        \"\"\""},{"line_number":1010,"context_line":"        old_metadata \u003d self.metadata"},{"line_number":1011,"context_line":"        if set(metadata_updates).issubset(set(old_metadata)):"}],"source_content_type":"text/x-python","patch_set":5,"id":"e3d57e45_0c0e7bbe","side":"PARENT","line":1008,"range":{"start_line":1003,"start_character":52,"end_line":1008,"end_character":23},"updated":"2025-05-16 23:24:30.000000000","message":"We probably ought to keep this part of the docstring for `update_metadata_and_put_timestamp` -- just stick in in between the summary line and the `:param ...:`s.","commit_id":"05143a99f8f3c860a887e4eb49688aef1e7a4a78"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"80eb5acb060e67afaf0886c4e62554bdefb92e14","unresolved":true,"context_lines":[{"line_number":1165,"context_line":"            DeprecationWarning,"},{"line_number":1166,"context_line":"            stacklevel\u003d2)"},{"line_number":1167,"context_line":"        if timestamp is not None and not isinstance(timestamp, Timestamp):"},{"line_number":1168,"context_line":"            timestamp \u003d Timestamp(timestamp)"},{"line_number":1169,"context_line":"        return self.update_metadata_and_put_timestamp({},"},{"line_number":1170,"context_line":"                                                      timestamp,"},{"line_number":1171,"context_line":"                                                      False)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7ed89f99_79f4dd28","line":1168,"updated":"2025-05-16 23:24:30.000000000","message":"Might want this down in `update_metadata_and_put_timestamp`","commit_id":"f41265a952fcac15a3e4985023bd80b8f7104809"}],"swift/container/server.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"d6fe52921a4f601219c805f07fb9e1bc2c2933c1","unresolved":true,"context_lines":[{"line_number":496,"context_line":"                    broker.set_x_container_sync_points(-1, -1)"},{"line_number":497,"context_line":"            broker.update_metadata_and_put_timestamp(metadata, req_timestamp,"},{"line_number":498,"context_line":"                                                     validate_metadata\u003dTrue)"},{"line_number":499,"context_line":"            self._update_sync_store(broker, method)"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"    @public"},{"line_number":502,"context_line":"    @timing_stats()"}],"source_content_type":"text/x-python","patch_set":1,"id":"d74d2e75_619a4b79","line":499,"updated":"2025-03-31 00:10:13.000000000","message":"Instead of repeating here do you think we could DRY it out a little. Maybe _update_metadata could grow an optional put_timestamp param, and when set use your new method rather then a copy?\n\n```\n    def _update_metadata(self, req, broker,\n                                           req_timestamp, method,\n                                           update_put_timestamp\u003dFalse):\n        metadata \u003d {\n            wsgi_to_str(key): (wsgi_to_str(value), req_timestamp.internal)\n            for key, value in req.headers.items()\n            if key.lower() in self.save_headers\n            or is_sys_or_user_meta(\u0027container\u0027, key)}\n        if metadata:\n            if \u0027X-Container-Sync-To\u0027 in metadata:\n                if \u0027X-Container-Sync-To\u0027 not in broker.metadata or \\\n                        metadata[\u0027X-Container-Sync-To\u0027][0] !\u003d \\\n                        broker.metadata[\u0027X-Container-Sync-To\u0027][0]:\n                    broker.set_x_container_sync_points(-1, -1)\n            if update_put_timestamp:\n            \tbroker.update_metadata_and_put_timestamp(metadata, req_timestamp,\n                                                         validate_metadata\u003dTrue)\n            else:\n                broker.update_metadata(metadata, validate_metadata\u003dTrue)\n```","commit_id":"a4a71ec0be7c12657163754bff5195b47b9df76c"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"d6fe52921a4f601219c805f07fb9e1bc2c2933c1","unresolved":true,"context_lines":[{"line_number":1019,"context_line":"        broker \u003d self._get_container_broker(drive, part, account, container)"},{"line_number":1020,"context_line":"        if broker.is_deleted():"},{"line_number":1021,"context_line":"            return HTTPNotFound(request\u003dreq)"},{"line_number":1022,"context_line":"        if not config_true_value("},{"line_number":1023,"context_line":"                req.headers.get(\u0027x-backend-no-timestamp-update\u0027, False)):"},{"line_number":1024,"context_line":"            self._update_metadata_and_put_timestamp(req, broker,"},{"line_number":1025,"context_line":"                                                    req_timestamp, \u0027POST\u0027)"},{"line_number":1026,"context_line":"        else:"},{"line_number":1027,"context_line":"            self._update_metadata(req, broker, req_timestamp, \u0027POST\u0027)"},{"line_number":1028,"context_line":"        return HTTPNoContent(request\u003dreq)"},{"line_number":1029,"context_line":""},{"line_number":1030,"context_line":"    def __call__(self, env, start_response):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9431b4ea_978c5378","line":1027,"range":{"start_line":1022,"start_character":7,"end_line":1027,"end_character":69},"updated":"2025-03-31 00:10:13.000000000","message":"If we do something like the above we could simply this bit to something like:\n\n```\nself._update_metadata(\n    req, broker, req_timestamp, \u0027POST\u0027, not config_true_value(\n        req.headers.get(\u0027x-backend-no-timestamp-update\u0027, False)))\n```\n\nOr if you want to make it more readable:\n\n```\nupdate_put_timestamp \u003d not config_true_value(\n        req.headers.get(\u0027x-backend-no-timestamp-update\u0027, False))\nself._update_metadata(req, broker, req_timestamp, \u0027POST\u0027, update_put_timestamp)\n```","commit_id":"a4a71ec0be7c12657163754bff5195b47b9df76c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"80eb5acb060e67afaf0886c4e62554bdefb92e14","unresolved":true,"context_lines":[{"line_number":483,"context_line":"                broker.update_metadata_and_put_timestamp("},{"line_number":484,"context_line":"                    metadata, req_timestamp, validate_metadata\u003dTrue)"},{"line_number":485,"context_line":"            else:"},{"line_number":486,"context_line":"                broker.update_metadata(metadata, validate_metadata\u003dTrue)"},{"line_number":487,"context_line":"            self._update_sync_store(broker, method)"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"    @public"}],"source_content_type":"text/x-python","patch_set":5,"id":"a893d1cc_351d4c8d","line":486,"updated":"2025-05-16 23:24:30.000000000","message":"We could avoid the call to the deprecated method with something like\n```\nbroker.update_metadata_and_put_timestamp(\n    metadata,\n    req_timestamp if update_put_timestamp else None,\n    validate_metadata\u003dTrue)\n```","commit_id":"f41265a952fcac15a3e4985023bd80b8f7104809"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"80eb5acb060e67afaf0886c4e62554bdefb92e14","unresolved":true,"context_lines":[{"line_number":582,"context_line":"                                         req_timestamp.internal,"},{"line_number":583,"context_line":"                                         new_container_policy,"},{"line_number":584,"context_line":"                                         requested_policy_index)"},{"line_number":585,"context_line":"        self._update_metadata(req, broker, req_timestamp, \u0027PUT\u0027)"},{"line_number":586,"context_line":"        resp \u003d self.account_update(req, account, container, broker)"},{"line_number":587,"context_line":"        if resp:"},{"line_number":588,"context_line":"            return resp"}],"source_content_type":"text/x-python","patch_set":5,"id":"f696ad7d_13030870","line":585,"updated":"2025-05-16 23:24:30.000000000","message":"I was kind of surprised that we wouldn\u0027t want to pass `update_put_timestamp\u003dTrue` here (or have that be the default, and explicitly pass `False` in `PUT_shard`), but I guess we already set the timestamp in `_update_or_create`...\n\nOh, but maybe we should be passing the metadata to `_update_or_create` and call `update_metadata_and_put_timestamp` there instead of (the now-deprecated) `update_put_timestamp`!","commit_id":"f41265a952fcac15a3e4985023bd80b8f7104809"}]}