)]}'
{"glance_store/_drivers/rbd.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"780ef6d6aa9640467677e5904e4b1e8eb46f8560","unresolved":false,"context_lines":[{"line_number":471,"context_line":""},{"line_number":472,"context_line":"    def _resize_on_write(self, image):"},{"line_number":473,"context_line":"        \"\"\"Handle the rbd resize when needed.\"\"\""},{"line_number":474,"context_line":"        new_size \u003d self.size + self.resize_amount"},{"line_number":475,"context_line":"        LOG.debug(\"resizing image to %s KiB\" % (new_size / units.Ki))"},{"line_number":476,"context_line":"        image.resize(new_size)"},{"line_number":477,"context_line":"        if self.resize_amount \u003c\u003d 4 * units.Gi:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_1264d5e4","line":474,"range":{"start_line":474,"start_character":31,"end_line":474,"end_character":49},"updated":"2020-08-17 19:47:40.000000000","message":"This is set later than object init. Right now it can\u0027t be called before it\u0027s set, but to future-proof this it seems like setting it in init, or changing this to:\n\n getattr(self, \u0027resize_amount\u0027, self.chunk_size)\n\nand removing the initialization on L535 might be more robust?","commit_id":"2f7da62ef657d3009d06c72f79664a522f171fe5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"780ef6d6aa9640467677e5904e4b1e8eb46f8560","unresolved":false,"context_lines":[{"line_number":474,"context_line":"        new_size \u003d self.size + self.resize_amount"},{"line_number":475,"context_line":"        LOG.debug(\"resizing image to %s KiB\" % (new_size / units.Ki))"},{"line_number":476,"context_line":"        image.resize(new_size)"},{"line_number":477,"context_line":"        if self.resize_amount \u003c\u003d 4 * units.Gi:"},{"line_number":478,"context_line":"            # Note(jokke): We double how much we grow the image each time"},{"line_number":479,"context_line":"            # up to 8gigs to avoid resizing for each write on bigger images"},{"line_number":480,"context_line":"            self.resize_amount *\u003d 2"},{"line_number":481,"context_line":"        elif self.resize_amount !\u003d 8 * units.Gi:"},{"line_number":482,"context_line":"            self.resize_amount \u003d 8 * units.Gi"},{"line_number":483,"context_line":"        return new_size"},{"line_number":484,"context_line":""},{"line_number":485,"context_line":"    @driver.back_compat_add"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_f2568106","line":482,"range":{"start_line":477,"start_character":0,"end_line":482,"end_character":45},"updated":"2020-08-17 19:47:40.000000000","message":"Wouldn\u0027t it be easier and more readable to replace all of this (and the increment on L474) with:\n\n self.resize_amount \u003d min(self.resize_amount, 8 * units.Gi)","commit_id":"2f7da62ef657d3009d06c72f79664a522f171fe5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"780ef6d6aa9640467677e5904e4b1e8eb46f8560","unresolved":false,"context_lines":[{"line_number":553,"context_line":"                            # ramp up to 8 gigs."},{"line_number":554,"context_line":"                            chunk_length \u003d len(chunk)"},{"line_number":555,"context_line":"                            if image_size \u003d\u003d 0 and (self.size \u003c bytes_written +"},{"line_number":556,"context_line":"                                                    chunk_length):"},{"line_number":557,"context_line":"                                self.size \u003d self._resize_on_write(image)"},{"line_number":558,"context_line":"                            LOG.debug(_(\"writing chunk at offset %s\") %"},{"line_number":559,"context_line":"                                      (offset))"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_528c8d7e","line":556,"updated":"2020-08-17 19:47:40.000000000","message":"Just my preference, but you could move this logic to the helper and just always call it to reduce the fairly crazy indent level here.","commit_id":"2f7da62ef657d3009d06c72f79664a522f171fe5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"b8def5b4fb7b2211a2f19de8a9af5013622709da","unresolved":false,"context_lines":[{"line_number":478,"context_line":"        new_size \u003d self.size + self.resize_amount"},{"line_number":479,"context_line":"        LOG.debug(\"resizing image to %s KiB\" % (new_size / units.Ki))"},{"line_number":480,"context_line":"        image.resize(new_size)"},{"line_number":481,"context_line":"        self.resize_amount *\u003d 2"},{"line_number":482,"context_line":"        # Note(jokke): We double how much we grow the image each time"},{"line_number":483,"context_line":"        # up to 8gigs to avoid resizing for each write on bigger images"},{"line_number":484,"context_line":"        self.resize_amount \u003d min(self.resize_amount, 8 * units.Gi)"},{"line_number":485,"context_line":"        return new_size"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"    @driver.back_compat_add"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_2e5408ca","line":484,"range":{"start_line":481,"start_character":8,"end_line":484,"end_character":66},"updated":"2020-08-18 14:43:19.000000000","message":"This looks little bit odd as we are calculating self.resize two times where earlier one is overwritten as well as little bit difficult to understand than if elif conditions used in earlier patch set.","commit_id":"3ef67618448ce7d796162d5587d464c98ba8e96b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"83d9780b121141569705a801c0785284f571efd2","unresolved":false,"context_lines":[{"line_number":478,"context_line":"        new_size \u003d self.size + self.resize_amount"},{"line_number":479,"context_line":"        LOG.debug(\"resizing image to %s KiB\" % (new_size / units.Ki))"},{"line_number":480,"context_line":"        image.resize(new_size)"},{"line_number":481,"context_line":"        self.resize_amount *\u003d 2"},{"line_number":482,"context_line":"        # Note(jokke): We double how much we grow the image each time"},{"line_number":483,"context_line":"        # up to 8gigs to avoid resizing for each write on bigger images"},{"line_number":484,"context_line":"        self.resize_amount \u003d min(self.resize_amount, 8 * units.Gi)"},{"line_number":485,"context_line":"        return new_size"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"    @driver.back_compat_add"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_8ea1d449","line":484,"range":{"start_line":481,"start_character":8,"end_line":484,"end_character":66},"in_reply_to":"9f560f44_2e5408ca","updated":"2020-08-18 14:50:12.000000000","message":"Yeah, just make this:\n\n min(self.resize_amount * 2, 8 * units.Gi)\n\nand you can collapse this to a single line of calculation.","commit_id":"3ef67618448ce7d796162d5587d464c98ba8e96b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4b3165cf2ffd4487e7806d81680cf451851f44f7","unresolved":false,"context_lines":[{"line_number":478,"context_line":"        new_size \u003d self.size + self.resize_amount"},{"line_number":479,"context_line":"        LOG.debug(\"resizing image to %s KiB\" % (new_size / units.Ki))"},{"line_number":480,"context_line":"        image.resize(new_size)"},{"line_number":481,"context_line":"        # Note(jokke): We double how much we grow the image each time"},{"line_number":482,"context_line":"        # up to 8gigs to avoid resizing for each write on bigger images"},{"line_number":483,"context_line":"        self.resize_amount \u003d min(self.resize_amount * 2, 8 * units.Gi)"},{"line_number":484,"context_line":"        return new_size"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_df3a54cb","line":481,"range":{"start_line":481,"start_character":10,"end_line":481,"end_character":21},"updated":"2020-08-21 05:45:07.000000000","message":"just nit\nNOTE(jokke):","commit_id":"a803988a880233c043b3736f2649ef248c03503d"},{"author":{"_account_id":26498,"name":"liumengke","email":"liumk@rc.inesa.com","username":"liumk"},"change_message_id":"dca55dd96d5cd624e8cb4ee4d3b832f8ad26c46f","unresolved":false,"context_lines":[{"line_number":485,"context_line":""},{"line_number":486,"context_line":"    @driver.back_compat_add"},{"line_number":487,"context_line":"    @capabilities.check"},{"line_number":488,"context_line":"    def add(self, image_id, image_file, image_size, hashing_algo, context\u003dNone,"},{"line_number":489,"context_line":"            verifier\u003dNone):"},{"line_number":490,"context_line":"        \"\"\""},{"line_number":491,"context_line":"        Stores an image file with supplied identifier to the backend"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_188481d2","line":488,"range":{"start_line":488,"start_character":12,"end_line":488,"end_character":16},"updated":"2020-08-25 03:02:32.000000000","message":"I have a question about this function.\nWhy not use \u0027rbd import\u0027 command in this function?\n\nIf using \u0027image.write\u0027 for image file in raw format (sparse file) block by block, the rbd device in images pool is very large, and its size is equal to \u0027virtual size\u0027 of image file.\nBut if we use \u0027rbd import\u0027 for image file in raw format, the rbd device in imgaes pool is very small, and its size is equal to \u0027disk size\u0027 of image file, and the upload  speed is fast.","commit_id":"c43f19e8456b9e20f03709773fb2ffdb94807a0a"}],"glance_store/tests/unit/test_rbd_store.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"780ef6d6aa9640467677e5904e4b1e8eb46f8560","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                                                self.conf)"},{"line_number":188,"context_line":"        # Provide enough data for few writes."},{"line_number":189,"context_line":"        self.data_len \u003d 57 * units.Mi"},{"line_number":190,"context_line":"        self.data_iter \u003d six.BytesIO(b\u0027*\u0027 * self.data_len)"},{"line_number":191,"context_line":"        self.hash_algo \u003d \u0027sha256\u0027"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def test_add_w_image_size_zero_less_resizes(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_574abfab","line":190,"updated":"2020-08-17 19:47:40.000000000","message":"This makes the unit tests swell by 57*2 \u003d 114MiB currently, and more if you add test cases here. Is there some reason you can\u0027t just produce this, like I do here?\n\nhttps://review.opendev.org/#/c/745392/4/glance/tests/functional/v2/test_images_import_locking.py@295\n\nMaybe this ^ FakeData thing should be a test util we can use elsewhere when we need to generate a lot of data without storing it. Seems like something glance likely needs to do a lot in tests.","commit_id":"2f7da62ef657d3009d06c72f79664a522f171fe5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"780ef6d6aa9640467677e5904e4b1e8eb46f8560","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                ret \u003d self.store.add("},{"line_number":198,"context_line":"                    \u0027fake_image_id\u0027, self.data_iter, 0, self.hash_algo)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"                self.assertEqual(5, resize.call_count)"},{"line_number":201,"context_line":"                self.assertEqual(8, write.call_count)"},{"line_number":202,"context_line":"                self.assertEqual(ret[1], self.data_len)"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_b7efdb85","line":200,"updated":"2020-08-17 19:47:40.000000000","message":"Can you assert the actual sizes used here so that it\u0027s easier to visually confirm that it looks like we expect?\n\n resize.assert_has_calls([mock.call(123), mock.call(246), ...])\n\nThat would help catch something in the future where the incrementing was broken by a refactor. It also helps verify that the resize-at-end actually gets called with what you expect.","commit_id":"2f7da62ef657d3009d06c72f79664a522f171fe5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"780ef6d6aa9640467677e5904e4b1e8eb46f8560","unresolved":false,"context_lines":[{"line_number":198,"context_line":"                    \u0027fake_image_id\u0027, self.data_iter, 0, self.hash_algo)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"                self.assertEqual(5, resize.call_count)"},{"line_number":201,"context_line":"                self.assertEqual(8, write.call_count)"},{"line_number":202,"context_line":"                self.assertEqual(ret[1], self.data_len)"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    def test_resize_on_write_cap(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_370d8b3e","line":201,"updated":"2020-08-17 19:47:40.000000000","message":"Also here, my thought was that there should be one less write than resize, and it wasn\u0027t clear until I read the code again why that wasn\u0027t the case. Might be more obvious and durable if this included details about the writes themselves, which is fairly easy to do (untested, but close):\n\n  # 56MiB in 8MiB chunks and one more after that to make 57MiB\n  chunk \u003d 8 * units.Mi\n  expected \u003d [(self.data_len / chunk) * units.Mi] + [(self.data_len % chunk) * units.Mi]\n  actual \u003d [len(args[0]) for args, kwargs in write.call_args_list]\n  self.assertEqual(expected, actual)","commit_id":"2f7da62ef657d3009d06c72f79664a522f171fe5"}]}
