)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ba9c2d161767182d3746b2699419d0d7644c606c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"0e5b5fa9_8108eb63","updated":"2024-07-29 10:34:50.000000000","message":"thanks for the new tests!\n\nI don\u0027t think of the variable naming in the same way, but nobody does when it comes to naming 😄","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"}],"swift/obj/expirer.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ba9c2d161767182d3746b2699419d0d7644c606c","unresolved":true,"context_lines":[{"line_number":166,"context_line":"            account_name, task_container)"},{"line_number":167,"context_line":"        return part, nodes, task_container"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def get_all_task_containers_per_day(self, task_container_ints):"},{"line_number":170,"context_line":"        \"\"\""},{"line_number":171,"context_line":"        :params task_container_ints: a list of ints, all task_containers taken"},{"line_number":172,"context_line":"                                     from listing of the expiring objects"}],"source_content_type":"text/x-python","patch_set":4,"id":"8d10a924_e0f0b188","line":169,"updated":"2024-07-29 10:34:50.000000000","message":"+1 the method returns both expected and unexpected containers so the rename makes sense","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"efb87b22d78ab976a1c97f8fda19788ea4401f54","unresolved":false,"context_lines":[{"line_number":166,"context_line":"            account_name, task_container)"},{"line_number":167,"context_line":"        return part, nodes, task_container"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def get_all_task_containers_per_day(self, task_container_ints):"},{"line_number":170,"context_line":"        \"\"\""},{"line_number":171,"context_line":"        :params task_container_ints: a list of ints, all task_containers taken"},{"line_number":172,"context_line":"                                     from listing of the expiring objects"}],"source_content_type":"text/x-python","patch_set":4,"id":"7749c57d_5151f92c","line":169,"in_reply_to":"8d10a924_e0f0b188","updated":"2024-07-30 05:11:18.000000000","message":"Acknowledged","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ba9c2d161767182d3746b2699419d0d7644c606c","unresolved":true,"context_lines":[{"line_number":619,"context_line":""},{"line_number":620,"context_line":"        :param container_list: List of task containers to be processed"},{"line_number":621,"context_line":"        :return: Tuple of (process_index, divisor, my_containers)"},{"line_number":622,"context_line":"            process_index: The index of this process within its group;"},{"line_number":623,"context_line":"            divisor: Number of processes in this process\u0027s group;"},{"line_number":624,"context_line":"            my_containers: List of containers this process or its process group"},{"line_number":625,"context_line":"            should iterate through and share delete tasks from."}],"source_content_type":"text/x-python","patch_set":4,"id":"0dc1c74e_a009b832","line":622,"range":{"start_line":622,"start_character":12,"end_line":622,"end_character":25},"updated":"2024-07-29 10:34:50.000000000","message":"Usually we\u0027d describe the tuple composition in terms of variables in the method; ``process_index`` isn\u0027t a variable. \n\nThe caller uses ``my_index``. I\u0027m not advocating that is a *great* name, but I think it helps to be consistent through the call tree where we can.\n\nFWIW if we did choose to rename everything I wonder if it would be helpful to use slice/range style naming i.e. ``start`` \u0026 ``step`` rather than ``my_index`` and ``divisor``. BUT, we should probably not get distracted by naming too much until we\u0027ve experimented with the strategy enough to like it 😊","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"efb87b22d78ab976a1c97f8fda19788ea4401f54","unresolved":false,"context_lines":[{"line_number":619,"context_line":""},{"line_number":620,"context_line":"        :param container_list: List of task containers to be processed"},{"line_number":621,"context_line":"        :return: Tuple of (process_index, divisor, my_containers)"},{"line_number":622,"context_line":"            process_index: The index of this process within its group;"},{"line_number":623,"context_line":"            divisor: Number of processes in this process\u0027s group;"},{"line_number":624,"context_line":"            my_containers: List of containers this process or its process group"},{"line_number":625,"context_line":"            should iterate through and share delete tasks from."}],"source_content_type":"text/x-python","patch_set":4,"id":"b9bf1186_f930ddcd","line":622,"range":{"start_line":622,"start_character":12,"end_line":622,"end_character":25},"in_reply_to":"0dc1c74e_a009b832","updated":"2024-07-30 05:11:18.000000000","message":"changed ``process_index`` in the comments to ``my_index``.","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ba9c2d161767182d3746b2699419d0d7644c606c","unresolved":true,"context_lines":[{"line_number":621,"context_line":"        :return: Tuple of (process_index, divisor, my_containers)"},{"line_number":622,"context_line":"            process_index: The index of this process within its group;"},{"line_number":623,"context_line":"            divisor: Number of processes in this process\u0027s group;"},{"line_number":624,"context_line":"            my_containers: List of containers this process or its process group"},{"line_number":625,"context_line":"            should iterate through and share delete tasks from."},{"line_number":626,"context_line":"        \"\"\""},{"line_number":627,"context_line":"        task_container_per_day \u003d self.expirer_config.task_container_per_day"}],"source_content_type":"text/x-python","patch_set":4,"id":"038aecb2_45d21bc1","line":624,"range":{"start_line":624,"start_character":59,"end_line":624,"end_character":79},"updated":"2024-07-29 10:34:50.000000000","message":"IMHO the \u0027or its process group\u0027 isn\u0027t necessary or relevant - the list of containers is always iterated by *this process*, there is no \u0027or\u0027 case.","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"efb87b22d78ab976a1c97f8fda19788ea4401f54","unresolved":false,"context_lines":[{"line_number":621,"context_line":"        :return: Tuple of (process_index, divisor, my_containers)"},{"line_number":622,"context_line":"            process_index: The index of this process within its group;"},{"line_number":623,"context_line":"            divisor: Number of processes in this process\u0027s group;"},{"line_number":624,"context_line":"            my_containers: List of containers this process or its process group"},{"line_number":625,"context_line":"            should iterate through and share delete tasks from."},{"line_number":626,"context_line":"        \"\"\""},{"line_number":627,"context_line":"        task_container_per_day \u003d self.expirer_config.task_container_per_day"}],"source_content_type":"text/x-python","patch_set":4,"id":"f487cc4b_88e7b221","line":624,"range":{"start_line":624,"start_character":59,"end_line":624,"end_character":79},"in_reply_to":"038aecb2_45d21bc1","updated":"2024-07-30 05:11:18.000000000","message":"I wanted to emphasize that the delete tasks in ``my_containers`` will be shared by all processes within the process group. changed to ``List of containers this process should iterate through and share delete tasks within its process group.``","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ba9c2d161767182d3746b2699419d0d7644c606c","unresolved":true,"context_lines":[{"line_number":630,"context_line":"            # each process maps to one task container per day, and handles a"},{"line_number":631,"context_line":"            # 1/divisor tasks in that container; each container maps to one or"},{"line_number":632,"context_line":"            # more processes."},{"line_number":633,"context_line":"            process_group_index \u003d self.process % task_container_per_day"},{"line_number":634,"context_line":"            my_index \u003d self.process // task_container_per_day"},{"line_number":635,"context_line":"            divisor \u003d self.processes // task_container_per_day"},{"line_number":636,"context_line":"            if process_group_index \u003c self.processes % task_container_per_day:"}],"source_content_type":"text/x-python","patch_set":4,"id":"45390c8c_23ba75f5","line":633,"range":{"start_line":633,"start_character":12,"end_line":633,"end_character":31},"updated":"2024-07-29 10:34:50.000000000","message":"everyone will have a different opinion on naming, but I preferred the original: \n\n(1) we\u0027re constructing a slice of the container list so the start of the slice is an index into the containers, not the processes.\n\n(2) I don\u0027t feel the concept of \u0027process_group\u0027 is clearly defined, but nor do I  feel I need the concept. When I read this method I\u0027m thinking about _containers_ for _one_ process, so I don\u0027t find it helpful to have to expand thinking to the concept of a \"process group\" that this process belongs to. This is about what this one process should do, and I found it more helpful to be thinking about container indexes as I build up the slice for this one process.\n\nActually, with hindsight I think I might prefer ``start_container_index``. 😄","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"efb87b22d78ab976a1c97f8fda19788ea4401f54","unresolved":true,"context_lines":[{"line_number":630,"context_line":"            # each process maps to one task container per day, and handles a"},{"line_number":631,"context_line":"            # 1/divisor tasks in that container; each container maps to one or"},{"line_number":632,"context_line":"            # more processes."},{"line_number":633,"context_line":"            process_group_index \u003d self.process % task_container_per_day"},{"line_number":634,"context_line":"            my_index \u003d self.process // task_container_per_day"},{"line_number":635,"context_line":"            divisor \u003d self.processes // task_container_per_day"},{"line_number":636,"context_line":"            if process_group_index \u003c self.processes % task_container_per_day:"}],"source_content_type":"text/x-python","patch_set":4,"id":"43ff82ab_c7b6145a","line":633,"range":{"start_line":633,"start_character":12,"end_line":633,"end_character":31},"in_reply_to":"45390c8c_23ba75f5","updated":"2024-07-30 05:11:18.000000000","message":"I feel process_group can help to visualize this new parallel strategy, that\u0027s how this idea of new strategy came to me as well: https://review.opendev.org/c/openstack/swift/+/918366/comment/ed646d67_63d49257/ since in a typical cluster, number of total processes will be more than a day of 100 containers, then a group of processes will share one particular container.\n\nIn the first implementation from Clay, he also used ``process_group``.\nhttps://review.opendev.org/c/openstack/swift/+/918366/9/swift/obj/expirer.py#554\n\nI added a new sentence in docstring to define this concept.","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"}],"test/unit/obj/test_expirer.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ba9c2d161767182d3746b2699419d0d7644c606c","unresolved":true,"context_lines":[{"line_number":359,"context_line":"        self.assertIn(\u0027ExpirerConfig\u0027, str(ctx.exception))"},{"line_number":360,"context_line":"        self.assertIn(\u0027container_ring kwarg\u0027, str(ctx.exception))"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"    def test_get_all_task_containers_per_day_only_expected(self):"},{"line_number":363,"context_line":"        # taks containers per day is 3"},{"line_number":364,"context_line":"        expirer_config \u003d ExpirerConfig({"},{"line_number":365,"context_line":"            \u0027expiring_objects_task_container_per_day\u0027: 3,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f9c7df7_a35d25bc","line":362,"updated":"2024-07-29 10:34:50.000000000","message":"thank you! this test would have really helped me when I was putting together the parallel_strategy tests.","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"efb87b22d78ab976a1c97f8fda19788ea4401f54","unresolved":false,"context_lines":[{"line_number":359,"context_line":"        self.assertIn(\u0027ExpirerConfig\u0027, str(ctx.exception))"},{"line_number":360,"context_line":"        self.assertIn(\u0027container_ring kwarg\u0027, str(ctx.exception))"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"    def test_get_all_task_containers_per_day_only_expected(self):"},{"line_number":363,"context_line":"        # taks containers per day is 3"},{"line_number":364,"context_line":"        expirer_config \u003d ExpirerConfig({"},{"line_number":365,"context_line":"            \u0027expiring_objects_task_container_per_day\u0027: 3,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a43d8d3_e786919c","line":362,"in_reply_to":"5f9c7df7_a35d25bc","updated":"2024-07-30 05:11:18.000000000","message":"Acknowledged","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ba9c2d161767182d3746b2699419d0d7644c606c","unresolved":true,"context_lines":[{"line_number":366,"context_line":"        })"},{"line_number":367,"context_line":"        containers \u003d [86398]"},{"line_number":368,"context_line":"        self.assertEqual("},{"line_number":369,"context_line":"            # within each day, task containers are in descending order."},{"line_number":370,"context_line":"            [[86400, 86399, 86398]],"},{"line_number":371,"context_line":"            expirer_config.get_all_task_containers_per_day(containers)"},{"line_number":372,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":4,"id":"31f5301a_6f1f2331","line":369,"updated":"2024-07-29 10:34:50.000000000","message":"right!","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"efb87b22d78ab976a1c97f8fda19788ea4401f54","unresolved":false,"context_lines":[{"line_number":366,"context_line":"        })"},{"line_number":367,"context_line":"        containers \u003d [86398]"},{"line_number":368,"context_line":"        self.assertEqual("},{"line_number":369,"context_line":"            # within each day, task containers are in descending order."},{"line_number":370,"context_line":"            [[86400, 86399, 86398]],"},{"line_number":371,"context_line":"            expirer_config.get_all_task_containers_per_day(containers)"},{"line_number":372,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":4,"id":"d98e1ba1_c3977b55","line":369,"in_reply_to":"31f5301a_6f1f2331","updated":"2024-07-30 05:11:18.000000000","message":"Acknowledged","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ba9c2d161767182d3746b2699419d0d7644c606c","unresolved":true,"context_lines":[{"line_number":429,"context_line":"        )"},{"line_number":430,"context_line":"        containers \u003d [86393, 86397, 86394]"},{"line_number":431,"context_line":"        self.assertEqual("},{"line_number":432,"context_line":"            # expect no sorting for unexpected task containers"},{"line_number":433,"context_line":"            [[86393, 86397, 86394]],"},{"line_number":434,"context_line":"            expirer_config.get_all_task_containers_per_day(containers)"},{"line_number":435,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":4,"id":"d4fe69d2_e0d33fff","line":432,"updated":"2024-07-29 10:34:50.000000000","message":"right! I think I missed that nuance before","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"efb87b22d78ab976a1c97f8fda19788ea4401f54","unresolved":false,"context_lines":[{"line_number":429,"context_line":"        )"},{"line_number":430,"context_line":"        containers \u003d [86393, 86397, 86394]"},{"line_number":431,"context_line":"        self.assertEqual("},{"line_number":432,"context_line":"            # expect no sorting for unexpected task containers"},{"line_number":433,"context_line":"            [[86393, 86397, 86394]],"},{"line_number":434,"context_line":"            expirer_config.get_all_task_containers_per_day(containers)"},{"line_number":435,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":4,"id":"0343de40_8c5b5cdf","line":432,"in_reply_to":"d4fe69d2_e0d33fff","updated":"2024-07-30 05:11:18.000000000","message":"Acknowledged","commit_id":"272616a7d00adbc8272e1aba3b8b5faa102f7c05"}]}