)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4fc3be61df8b4a80840ceb142163b2b3451e9ab5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"96bc4419_ca2b520c","updated":"2022-09-08 11:37:17.000000000","message":"I pushed my alternative idea here https://review.opendev.org/c/openstack/swift/+/856484 ","commit_id":"32cb5b1b631b0221a46d90ac1245e3078c70dd12"}],"test/probe/test_sharder.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"0e30aa05276028e2380ecdf2053b6b47f8768499","unresolved":true,"context_lines":[{"line_number":432,"context_line":"        return self.run_custom_daemon(ContainerSharder, \u0027container-sharder\u0027,"},{"line_number":433,"context_line":"                                      conf_index, custom_conf, **kwargs)"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"    def nodes_without_index(self, nodes, *indexes):"},{"line_number":436,"context_line":"        for node in nodes:"},{"line_number":437,"context_line":"            match_node \u003d {k: mock.ANY if k \u003d\u003d \u0027index\u0027 else v"},{"line_number":438,"context_line":"                          for k, v in node.items()}"}],"source_content_type":"text/x-python","patch_set":2,"id":"51a03d83_489c4f66","line":435,"updated":"2022-09-07 05:31:29.000000000","message":"this new function is cool, but I feel original \"pop_index\" function is much easier to read/understand.","commit_id":"32cb5b1b631b0221a46d90ac1245e3078c70dd12"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"84cd117ffbbb2cbd2933f9d598e096c60fe8607d","unresolved":true,"context_lines":[{"line_number":432,"context_line":"        return self.run_custom_daemon(ContainerSharder, \u0027container-sharder\u0027,"},{"line_number":433,"context_line":"                                      conf_index, custom_conf, **kwargs)"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"    def nodes_without_index(self, nodes, *indexes):"},{"line_number":436,"context_line":"        for node in nodes:"},{"line_number":437,"context_line":"            match_node \u003d {k: mock.ANY if k \u003d\u003d \u0027index\u0027 else v"},{"line_number":438,"context_line":"                          for k, v in node.items()}"}],"source_content_type":"text/x-python","patch_set":2,"id":"a0b4965d_ad1559fa","line":435,"in_reply_to":"51a03d83_489c4f66","updated":"2022-09-07 12:10:05.000000000","message":"I found the method name confusing - at first I thought it was going to yield node dicts without the index keys.\n\nI think it at least needs a docstring that clarifies what indexes refers to: it\u0027s yielding nodes that don\u0027t match brain nodes with given indexes. The indexes are actually the concern of the brain, so maybe it should be a method of brain??\n\nOr, how about changing the method name and signature to make it a bit more explicit:\n\n  def filter_nodes(nodes, exclude_nodes):\n      for node in nodes:\n          # we ignore the index key when comparing nodes because index\n          # varies with the partition for which the nodes were generated\n          match_node \u003d {k: mock.ANY if k \u003d\u003d \u0027index\u0027 else v\n                        for k, v in node.items()}\n          if any(n \u003d\u003d match_node for n in exclude_nodes):\n              continue\n          yield node\n          \nThat ^^ would of course be a module function!\n\nIt has the advantage of being more flexible (e.g. if we ever want to exclude a shard node from a list of brain nodes).","commit_id":"32cb5b1b631b0221a46d90ac1245e3078c70dd12"}]}