)]}'
{"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b76035e68ad4fd75ee1d8b744909e41e15af6555","unresolved":false,"context_lines":[{"line_number":23537,"context_line":"    @mock.patch(\u0027os.path.exists\u0027)"},{"line_number":23538,"context_line":"    @mock.patch(\u0027os.utime\u0027)"},{"line_number":23539,"context_line":"    @mock.patch(\u0027nova.virt.images.fetch_to_raw\u0027)"},{"line_number":23540,"context_line":"    def test_cache_image_uncached(self, mock_fetch, mock_utime, mock_exists,"},{"line_number":23541,"context_line":"                                  mock_isdir, mock_mkdir, first_time\u003dFalse):"},{"line_number":23542,"context_line":"        self.flags(instances_path\u003d\u0027/nova/instances\u0027)"},{"line_number":23543,"context_line":"        self.flags(image_cache_subdirectory_name\u003d\u0027cache\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_c639773f","line":23540,"updated":"2019-10-08 14:02:09.000000000","message":"✓ this tests the \"else\" block in cache_image()","commit_id":"4c9693de63004eb378b539b3215e8c643d5b2a8d"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"5426ac3dd00c00111e3514b50f8bd0abac971d1a","unresolved":false,"context_lines":[{"line_number":23539,"context_line":"    @mock.patch(\u0027nova.virt.images.fetch_to_raw\u0027)"},{"line_number":23540,"context_line":"    def test_cache_image_uncached(self, mock_fetch, mock_utime, mock_exists,"},{"line_number":23541,"context_line":"                                  mock_isdir, mock_mkdir, first_time\u003dFalse):"},{"line_number":23542,"context_line":"        self.flags(instances_path\u003d\u0027/nova/instances\u0027)"},{"line_number":23543,"context_line":"        self.flags(image_cache_subdirectory_name\u003d\u0027cache\u0027)"},{"line_number":23544,"context_line":"        expected_fn \u003d os.path.join(\u0027/nova/instances/cache\u0027,"},{"line_number":23545,"context_line":"                                   imagecache.get_cache_fname(\u0027an-image\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_8dcba251","line":23542,"updated":"2019-10-08 11:41:48.000000000","message":"Is this... safe? Shouldn\u0027t we use a TempDir fixture here?","commit_id":"4c9693de63004eb378b539b3215e8c643d5b2a8d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2bd6934b381f1e8295a3f08cbf2c54fcd5db996f","unresolved":false,"context_lines":[{"line_number":23539,"context_line":"    @mock.patch(\u0027nova.virt.images.fetch_to_raw\u0027)"},{"line_number":23540,"context_line":"    def test_cache_image_uncached(self, mock_fetch, mock_utime, mock_exists,"},{"line_number":23541,"context_line":"                                  mock_isdir, mock_mkdir, first_time\u003dFalse):"},{"line_number":23542,"context_line":"        self.flags(instances_path\u003d\u0027/nova/instances\u0027)"},{"line_number":23543,"context_line":"        self.flags(image_cache_subdirectory_name\u003d\u0027cache\u0027)"},{"line_number":23544,"context_line":"        expected_fn \u003d os.path.join(\u0027/nova/instances/cache\u0027,"},{"line_number":23545,"context_line":"                                   imagecache.get_cache_fname(\u0027an-image\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_e627d363","line":23542,"in_reply_to":"3fa7e38b_8dcba251","updated":"2019-10-08 13:49:41.000000000","message":"It\u0027s not actually doing anything, this is just the path string we use to concatenate the later values. Using a TempDir would actually create and destroy the directory on the test machine for no reason.","commit_id":"4c9693de63004eb378b539b3215e8c643d5b2a8d"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b76035e68ad4fd75ee1d8b744909e41e15af6555","unresolved":false,"context_lines":[{"line_number":23539,"context_line":"    @mock.patch(\u0027nova.virt.images.fetch_to_raw\u0027)"},{"line_number":23540,"context_line":"    def test_cache_image_uncached(self, mock_fetch, mock_utime, mock_exists,"},{"line_number":23541,"context_line":"                                  mock_isdir, mock_mkdir, first_time\u003dFalse):"},{"line_number":23542,"context_line":"        self.flags(instances_path\u003d\u0027/nova/instances\u0027)"},{"line_number":23543,"context_line":"        self.flags(image_cache_subdirectory_name\u003d\u0027cache\u0027)"},{"line_number":23544,"context_line":"        expected_fn \u003d os.path.join(\u0027/nova/instances/cache\u0027,"},{"line_number":23545,"context_line":"                                   imagecache.get_cache_fname(\u0027an-image\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_66f3e39f","line":23542,"in_reply_to":"3fa7e38b_e627d363","updated":"2019-10-08 14:02:09.000000000","message":"Oh right, because you\u0027ve mocked out all the things that actually touch the filesystem. If you respin, maybe add a comment to that effect, or change the path to something more clearly fake, like \"/fake/instances/path\".","commit_id":"4c9693de63004eb378b539b3215e8c643d5b2a8d"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b76035e68ad4fd75ee1d8b744909e41e15af6555","unresolved":false,"context_lines":[{"line_number":23552,"context_line":"        mock_utime.assert_not_called()"},{"line_number":23553,"context_line":"        mock_exists.assert_called_once_with(expected_fn)"},{"line_number":23554,"context_line":"        mock_isdir.assert_called_once_with(\u0027/nova/instances/cache\u0027)"},{"line_number":23555,"context_line":"        if first_time:"},{"line_number":23556,"context_line":"            mock_mkdir.assert_called_once_with(\u0027/nova/instances/cache\u0027)"},{"line_number":23557,"context_line":"        else:"},{"line_number":23558,"context_line":"            mock_mkdir.assert_not_called()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_6621a3fb","line":23555,"updated":"2019-10-08 14:02:09.000000000","message":"✓ this tests going inside the \"if\" inside the \"else\" of cache_image()","commit_id":"4c9693de63004eb378b539b3215e8c643d5b2a8d"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b76035e68ad4fd75ee1d8b744909e41e15af6555","unresolved":false,"context_lines":[{"line_number":23555,"context_line":"        if first_time:"},{"line_number":23556,"context_line":"            mock_mkdir.assert_called_once_with(\u0027/nova/instances/cache\u0027)"},{"line_number":23557,"context_line":"        else:"},{"line_number":23558,"context_line":"            mock_mkdir.assert_not_called()"},{"line_number":23559,"context_line":""},{"line_number":23560,"context_line":"    def test_cache_image_existing_first_time(self):"},{"line_number":23561,"context_line":"        self.test_cache_image_uncached(first_time\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_467ca7ea","line":23558,"updated":"2019-10-08 14:02:09.000000000","message":"✓ this is for not going inside that \"if\"","commit_id":"4c9693de63004eb378b539b3215e8c643d5b2a8d"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b76035e68ad4fd75ee1d8b744909e41e15af6555","unresolved":false,"context_lines":[{"line_number":23557,"context_line":"        else:"},{"line_number":23558,"context_line":"            mock_mkdir.assert_not_called()"},{"line_number":23559,"context_line":""},{"line_number":23560,"context_line":"    def test_cache_image_existing_first_time(self):"},{"line_number":23561,"context_line":"        self.test_cache_image_uncached(first_time\u003dTrue)"},{"line_number":23562,"context_line":""},{"line_number":23563,"context_line":"    @mock.patch(\u0027os.mkdir\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_e68273c8","line":23560,"updated":"2019-10-08 14:02:09.000000000","message":"✓ and this is for the \"if\" in cache_image()","commit_id":"4c9693de63004eb378b539b3215e8c643d5b2a8d"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"59d0272928f2cbeddbfe05932e639b02a06c03fb","unresolved":false,"context_lines":[{"line_number":23571,"context_line":"    @mock.patch(\u0027nova.virt.images.fetch_to_raw\u0027)"},{"line_number":23572,"context_line":"    def test_cache_image_existing(self, mock_fetch, mock_utime, mock_exists,"},{"line_number":23573,"context_line":"                                  mock_isdir, mock_mkdir):"},{"line_number":23574,"context_line":"        # NOTE(artom): This is not actually a path on the system, since we"},{"line_number":23575,"context_line":"        # are fully mocked out and are just testing string formatting in this"},{"line_number":23576,"context_line":"        # test."},{"line_number":23577,"context_line":"        self.flags(instances_path\u003d\u0027/nova/instances\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_cb341b4d","line":23574,"range":{"start_line":23574,"start_character":15,"end_line":23574,"end_character":20},"updated":"2019-10-08 17:54:22.000000000","message":"Gee, thanks ;)","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5fa9286d54045c5615535908bed0b1630fc34938","unresolved":false,"context_lines":[{"line_number":23571,"context_line":"    @mock.patch(\u0027nova.virt.images.fetch_to_raw\u0027)"},{"line_number":23572,"context_line":"    def test_cache_image_existing(self, mock_fetch, mock_utime, mock_exists,"},{"line_number":23573,"context_line":"                                  mock_isdir, mock_mkdir):"},{"line_number":23574,"context_line":"        # NOTE(artom): This is not actually a path on the system, since we"},{"line_number":23575,"context_line":"        # are fully mocked out and are just testing string formatting in this"},{"line_number":23576,"context_line":"        # test."},{"line_number":23577,"context_line":"        self.flags(instances_path\u003d\u0027/nova/instances\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_f19464ea","line":23574,"range":{"start_line":23574,"start_character":15,"end_line":23574,"end_character":20},"in_reply_to":"3fa7e38b_cb341b4d","updated":"2019-10-08 19:18:13.000000000","message":"S\u0027not like I TODO(artom)\u0027d you :)","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d2a76aaa863d653f71791f77420c87d34e47e3dd","unresolved":false,"context_lines":[{"line_number":23561,"context_line":"        else:"},{"line_number":23562,"context_line":"            mock_et.assert_not_called()"},{"line_number":23563,"context_line":""},{"line_number":23564,"context_line":"    def test_cache_image_existing_first_time(self):"},{"line_number":23565,"context_line":"        self.test_cache_image_uncached(first_time\u003dTrue)"},{"line_number":23566,"context_line":""},{"line_number":23567,"context_line":"    @mock.patch(\u0027oslo_utils.fileutils.ensure_tree\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_89c352ed","line":23564,"range":{"start_line":23564,"start_character":8,"end_line":23564,"end_character":44},"updated":"2019-10-11 15:51:06.000000000","message":"Test case naming feels confusing to me. Nothing exists in this test neither the cache directory nor the image.","commit_id":"e46d59a7f7e9c75c777e7ef6a8bf32037acb0bb1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a8ae5311d9f75faf517ae93fedee3390fd010685","unresolved":false,"context_lines":[{"line_number":23561,"context_line":"        else:"},{"line_number":23562,"context_line":"            mock_et.assert_not_called()"},{"line_number":23563,"context_line":""},{"line_number":23564,"context_line":"    def test_cache_image_existing_first_time(self):"},{"line_number":23565,"context_line":"        self.test_cache_image_uncached(first_time\u003dTrue)"},{"line_number":23566,"context_line":""},{"line_number":23567,"context_line":"    @mock.patch(\u0027oslo_utils.fileutils.ensure_tree\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_e4246bc8","line":23564,"range":{"start_line":23564,"start_character":8,"end_line":23564,"end_character":44},"in_reply_to":"3fa7e38b_89c352ed","updated":"2019-10-11 16:05:58.000000000","message":"Yep, this is totally wrong, mah bad.","commit_id":"e46d59a7f7e9c75c777e7ef6a8bf32037acb0bb1"}],"nova/virt/driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9b4ca16fa222427194261c51f82f36d8b312a37e","unresolved":false,"context_lines":[{"line_number":1570,"context_line":"        \"\"\"Does the driver want networks deallocated on reschedule?\"\"\""},{"line_number":1571,"context_line":"        return False"},{"line_number":1572,"context_line":""},{"line_number":1573,"context_line":"    def manage_image_cache(self, context, all_instances):"},{"line_number":1574,"context_line":"        \"\"\"Manage the driver\u0027s local image cache."},{"line_number":1575,"context_line":""},{"line_number":1576,"context_line":"        Some drivers chose to cache images for instances on disk. This method"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_de292237","line":1573,"updated":"2019-10-09 18:37:10.000000000","message":"nit: I\u0027d throw the new method closer to this one.","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"5426ac3dd00c00111e3514b50f8bd0abac971d1a","unresolved":false,"context_lines":[{"line_number":10524,"context_line":"                os.mkdir(cache_dir)"},{"line_number":10525,"context_line":"            LOG.info(\u0027Caching image %(image_id)s by request\u0027,"},{"line_number":10526,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":10527,"context_line":"            images.fetch_to_raw(context, image_id, path)"},{"line_number":10528,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_cd83da03","line":10527,"updated":"2019-10-08 11:41:48.000000000","message":"This can raise ImageUnacceptable, but there\u0027s a pokemon exception catcher in the compute manahger in the patch on top.","commit_id":"4c9693de63004eb378b539b3215e8c643d5b2a8d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9b4ca16fa222427194261c51f82f36d8b312a37e","unresolved":false,"context_lines":[{"line_number":9627,"context_line":"        out, err \u003d processutils.execute(\u0027env\u0027, \u0027LANG\u003dC\u0027, \u0027uptime\u0027)"},{"line_number":9628,"context_line":"        return out"},{"line_number":9629,"context_line":""},{"line_number":9630,"context_line":"    def manage_image_cache(self, context, all_instances):"},{"line_number":9631,"context_line":"        \"\"\"Manage the local cache of images.\"\"\""},{"line_number":9632,"context_line":"        self.image_cache_manager.update(context, all_instances)"},{"line_number":9633,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_1e349a92","line":9630,"updated":"2019-10-09 18:37:10.000000000","message":"nit: I\u0027d throw the new method closer to this one.","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9b4ca16fa222427194261c51f82f36d8b312a37e","unresolved":false,"context_lines":[{"line_number":10508,"context_line":"                    return None"},{"line_number":10509,"context_line":""},{"line_number":10510,"context_line":"    def cache_image(self, context, image_id):"},{"line_number":10511,"context_line":"        cache_dir \u003d os.path.join(CONF.instances_path,"},{"line_number":10512,"context_line":"                                 CONF.image_cache_subdirectory_name)"},{"line_number":10513,"context_line":"        path \u003d os.path.join(cache_dir,"},{"line_number":10514,"context_line":"                            imagecache.get_cache_fname(image_id))"},{"line_number":10515,"context_line":"        if os.path.exists(path):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_7ed02e20","line":10512,"range":{"start_line":10511,"start_character":20,"end_line":10512,"end_character":68},"updated":"2019-10-09 18:37:10.000000000","message":"OK this matches:\n\nhttps://github.com/openstack/nova/blob/9a9d412ef8863d694736da1ba7247a0f47578bf9/nova/virt/libvirt/imagebackend.py#L251\n\nCould be a follow up to generalize that into a helper method like how imagecache.get_cache_fname works.","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9b4ca16fa222427194261c51f82f36d8b312a37e","unresolved":false,"context_lines":[{"line_number":10515,"context_line":"        if os.path.exists(path):"},{"line_number":10516,"context_line":"            LOG.info(\u0027Image %(image_id)s already cached; updating timestamp\u0027,"},{"line_number":10517,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":10518,"context_line":"            nova.privsep.path.utime(path)"},{"line_number":10519,"context_line":"            return False"},{"line_number":10520,"context_line":"        else:"},{"line_number":10521,"context_line":"            # NOTE(danms): In case we are running before the first boot, make"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_fe8ade18","line":10518,"range":{"start_line":10518,"start_character":30,"end_line":10518,"end_character":35},"updated":"2019-10-09 18:37:10.000000000","message":"Hmm, the image backend code seems to use this instead:\n\nhttps://github.com/openstack/nova/blob/9a9d412ef8863d694736da1ba7247a0f47578bf9/nova/virt/libvirt/imagebackend.py#L70\n\nSeems we should make that public and use it here also.","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4cabbd8b5474752e8e27c5eeb7ed5d469ef0a7f5","unresolved":false,"context_lines":[{"line_number":10515,"context_line":"        if os.path.exists(path):"},{"line_number":10516,"context_line":"            LOG.info(\u0027Image %(image_id)s already cached; updating timestamp\u0027,"},{"line_number":10517,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":10518,"context_line":"            nova.privsep.path.utime(path)"},{"line_number":10519,"context_line":"            return False"},{"line_number":10520,"context_line":"        else:"},{"line_number":10521,"context_line":"            # NOTE(danms): In case we are running before the first boot, make"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_41854359","line":10518,"range":{"start_line":10518,"start_character":30,"end_line":10518,"end_character":35},"in_reply_to":"3fa7e38b_0dfd446d","updated":"2019-10-10 15:42:14.000000000","message":"If you do we should leave a code comment I think otherwise I won\u0027t be surprised if someone tries to change it later.","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a5a512f301b4569f33fd4c90f48983d1a923d89e","unresolved":false,"context_lines":[{"line_number":10515,"context_line":"        if os.path.exists(path):"},{"line_number":10516,"context_line":"            LOG.info(\u0027Image %(image_id)s already cached; updating timestamp\u0027,"},{"line_number":10517,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":10518,"context_line":"            nova.privsep.path.utime(path)"},{"line_number":10519,"context_line":"            return False"},{"line_number":10520,"context_line":"        else:"},{"line_number":10521,"context_line":"            # NOTE(danms): In case we are running before the first boot, make"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_0dfd446d","line":10518,"range":{"start_line":10518,"start_character":30,"end_line":10518,"end_character":35},"in_reply_to":"3fa7e38b_fe317e0c","updated":"2019-10-10 14:09:02.000000000","message":"So, actually, that won\u0027t re-raise, which is what we want to happen so that the compute manager can report it. So I actually think I\u0027d rather just keep this bare. It\u0027s the same one functional line, just without the exception washing.","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3a089a8f3218d7108cfc898d863707d3a0c20e00","unresolved":false,"context_lines":[{"line_number":10515,"context_line":"        if os.path.exists(path):"},{"line_number":10516,"context_line":"            LOG.info(\u0027Image %(image_id)s already cached; updating timestamp\u0027,"},{"line_number":10517,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":10518,"context_line":"            nova.privsep.path.utime(path)"},{"line_number":10519,"context_line":"            return False"},{"line_number":10520,"context_line":"        else:"},{"line_number":10521,"context_line":"            # NOTE(danms): In case we are running before the first boot, make"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_fe317e0c","line":10518,"range":{"start_line":10518,"start_character":30,"end_line":10518,"end_character":35},"in_reply_to":"3fa7e38b_fe8ade18","updated":"2019-10-09 18:48:46.000000000","message":"Ack yep","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9b4ca16fa222427194261c51f82f36d8b312a37e","unresolved":false,"context_lines":[{"line_number":10517,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":10518,"context_line":"            nova.privsep.path.utime(path)"},{"line_number":10519,"context_line":"            return False"},{"line_number":10520,"context_line":"        else:"},{"line_number":10521,"context_line":"            # NOTE(danms): In case we are running before the first boot, make"},{"line_number":10522,"context_line":"            # sure the cache directory is created"},{"line_number":10523,"context_line":"            if not os.path.isdir(cache_dir):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_9ee40a0f","line":10520,"range":{"start_line":10520,"start_character":8,"end_line":10520,"end_character":13},"updated":"2019-10-09 18:37:10.000000000","message":"Remove this else since if you hit the if condition above you\u0027ve already returned.","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3a089a8f3218d7108cfc898d863707d3a0c20e00","unresolved":false,"context_lines":[{"line_number":10517,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":10518,"context_line":"            nova.privsep.path.utime(path)"},{"line_number":10519,"context_line":"            return False"},{"line_number":10520,"context_line":"        else:"},{"line_number":10521,"context_line":"            # NOTE(danms): In case we are running before the first boot, make"},{"line_number":10522,"context_line":"            # sure the cache directory is created"},{"line_number":10523,"context_line":"            if not os.path.isdir(cache_dir):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_5e53f239","line":10520,"range":{"start_line":10520,"start_character":8,"end_line":10520,"end_character":13},"in_reply_to":"3fa7e38b_9ee40a0f","updated":"2019-10-09 18:48:46.000000000","message":"I hate that style, FWIW :)","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9b4ca16fa222427194261c51f82f36d8b312a37e","unresolved":false,"context_lines":[{"line_number":10521,"context_line":"            # NOTE(danms): In case we are running before the first boot, make"},{"line_number":10522,"context_line":"            # sure the cache directory is created"},{"line_number":10523,"context_line":"            if not os.path.isdir(cache_dir):"},{"line_number":10524,"context_line":"                os.mkdir(cache_dir)"},{"line_number":10525,"context_line":"            LOG.info(\u0027Caching image %(image_id)s by request\u0027,"},{"line_number":10526,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":10527,"context_line":"            images.fetch_to_raw(context, image_id, path)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_be954634","line":10524,"updated":"2019-10-09 18:37:10.000000000","message":"Any reason to not use fileutils.ensure_tree here like in the image backend cache() method?\n\nhttps://github.com/openstack/nova/blob/9a9d412ef8863d694736da1ba7247a0f47578bf9/nova/virt/libvirt/imagebackend.py#L254","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9b4ca16fa222427194261c51f82f36d8b312a37e","unresolved":false,"context_lines":[{"line_number":10524,"context_line":"                os.mkdir(cache_dir)"},{"line_number":10525,"context_line":"            LOG.info(\u0027Caching image %(image_id)s by request\u0027,"},{"line_number":10526,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":10527,"context_line":"            images.fetch_to_raw(context, image_id, path)"},{"line_number":10528,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_feddfe13","line":10527,"updated":"2019-10-09 18:37:10.000000000","message":"Is there any need to use the same lock as here?\n\nhttps://github.com/openstack/nova/blob/9a9d412ef8863d694736da1ba7247a0f47578bf9/nova/virt/libvirt/imagebackend.py#L257","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a5a512f301b4569f33fd4c90f48983d1a923d89e","unresolved":false,"context_lines":[{"line_number":10524,"context_line":"                os.mkdir(cache_dir)"},{"line_number":10525,"context_line":"            LOG.info(\u0027Caching image %(image_id)s by request\u0027,"},{"line_number":10526,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":10527,"context_line":"            images.fetch_to_raw(context, image_id, path)"},{"line_number":10528,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_d03c9fc9","line":10527,"in_reply_to":"3fa7e38b_9e22cab8","updated":"2019-10-10 14:09:02.000000000","message":"Okay, sorry, I had to re-figure this all back out. I had this looked up for Artom week before last and took notes, but it\u0027s obscure and fell out of my head. So, the lock you\u0027ve linked to is a lock on the filename that will become the root disk of the instance being created. Since we\u0027re not doing that here, we have no such name to lock on. Further, two instance being booted at the same time that have the same image would be locked on their respective target disk names, not the common base image they both are about to try to fetch.\n\nThe actual place where they synchronize is in images.fetch() on compute_utils.disk_ops_semaphore:\n\nhttps://github.com/openstack/nova/blob/9a9d412ef8863d694736da1ba7247a0f47578bf9/nova/virt/images.py#L142\n\nwhich is a layer under fetch_to_raw() that I\u0027m calling here. So, we\u0027re generating a stable temp name (${image}.part) and then grabbing the semaphore before we ask the image_api to download to that filename. It must fail if the file exists when it goes to open it, otherwise we\u0027d race when booting lots of instances from the same image today.\n\nI can go check that image code to make sure it\u0027s doing what the existing code thinks it is, but at the end of the day, this is doing the same thing that spawn is.","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4cabbd8b5474752e8e27c5eeb7ed5d469ef0a7f5","unresolved":false,"context_lines":[{"line_number":10524,"context_line":"                os.mkdir(cache_dir)"},{"line_number":10525,"context_line":"            LOG.info(\u0027Caching image %(image_id)s by request\u0027,"},{"line_number":10526,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":10527,"context_line":"            images.fetch_to_raw(context, image_id, path)"},{"line_number":10528,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_c19853c1","line":10527,"in_reply_to":"3fa7e38b_d03c9fc9","updated":"2019-10-10 15:42:14.000000000","message":"OK if we don\u0027t need to worry about locks here to avoid colliding with the image cache do you mind adding some kind of comment about why we\u0027re not locking?","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3a089a8f3218d7108cfc898d863707d3a0c20e00","unresolved":false,"context_lines":[{"line_number":10524,"context_line":"                os.mkdir(cache_dir)"},{"line_number":10525,"context_line":"            LOG.info(\u0027Caching image %(image_id)s by request\u0027,"},{"line_number":10526,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":10527,"context_line":"            images.fetch_to_raw(context, image_id, path)"},{"line_number":10528,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_9e22cab8","line":10527,"in_reply_to":"3fa7e38b_feddfe13","updated":"2019-10-09 18:48:46.000000000","message":"Yeah, I meant to.","commit_id":"7337baa4e4791829e329ed6403189a81ae23b777"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8445590ed5446cee6e46288db0a0d9c2717d0bc","unresolved":false,"context_lines":[{"line_number":9651,"context_line":"        if os.path.exists(path):"},{"line_number":9652,"context_line":"            LOG.info(\u0027Image %(image_id)s already cached; updating timestamp\u0027,"},{"line_number":9653,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":9654,"context_line":"            nova.privsep.path.utime(path)"},{"line_number":9655,"context_line":"            return False"},{"line_number":9656,"context_line":"        else:"},{"line_number":9657,"context_line":"            # NOTE(danms): In case we are running before the first boot, make"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_21f5a7ed","line":9654,"updated":"2019-10-10 15:44:43.000000000","message":"Per PS2, can we add a comment about why we\u0027re not using _update_utime_ignore_eacces here?","commit_id":"5e5eedcd4f8f3b9969059a45a08a787284f658c8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b1b21e4749dee070b4b96b44a86c532c218fd237","unresolved":false,"context_lines":[{"line_number":9651,"context_line":"        if os.path.exists(path):"},{"line_number":9652,"context_line":"            LOG.info(\u0027Image %(image_id)s already cached; updating timestamp\u0027,"},{"line_number":9653,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":9654,"context_line":"            nova.privsep.path.utime(path)"},{"line_number":9655,"context_line":"            return False"},{"line_number":9656,"context_line":"        else:"},{"line_number":9657,"context_line":"            # NOTE(danms): In case we are running before the first boot, make"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_61907f4b","line":9654,"in_reply_to":"3fa7e38b_21f5a7ed","updated":"2019-10-10 15:59:27.000000000","message":"Okay, it\u0027s a little weird to add a comment about why we\u0027re not using something (I could write several such things here, other than just the one you pointed out) so I wasn\u0027t thinking it was really necessary. But happy to add it.","commit_id":"5e5eedcd4f8f3b9969059a45a08a787284f658c8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a854c7e40c2a345e26b4843b3e6a72118a5201fd","unresolved":false,"context_lines":[{"line_number":9651,"context_line":"        if os.path.exists(path):"},{"line_number":9652,"context_line":"            LOG.info(\u0027Image %(image_id)s already cached; updating timestamp\u0027,"},{"line_number":9653,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":9654,"context_line":"            nova.privsep.path.utime(path)"},{"line_number":9655,"context_line":"            return False"},{"line_number":9656,"context_line":"        else:"},{"line_number":9657,"context_line":"            # NOTE(danms): In case we are running before the first boot, make"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_611abfcc","line":9654,"in_reply_to":"3fa7e38b_61907f4b","updated":"2019-10-10 16:00:29.000000000","message":"Ah, I see you commented that request after I pushed PS3.","commit_id":"5e5eedcd4f8f3b9969059a45a08a787284f658c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8445590ed5446cee6e46288db0a0d9c2717d0bc","unresolved":false,"context_lines":[{"line_number":9660,"context_line":"                fileutils.ensure_tree(cache_dir)"},{"line_number":9661,"context_line":"            LOG.info(\u0027Caching image %(image_id)s by request\u0027,"},{"line_number":9662,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":9663,"context_line":"            images.fetch_to_raw(context, image_id, path)"},{"line_number":9664,"context_line":"            return True"},{"line_number":9665,"context_line":""},{"line_number":9666,"context_line":"    def _is_storage_shared_with(self, dest, inst_base):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_41d30361","line":9663,"updated":"2019-10-10 15:44:43.000000000","message":"Per PS2, can we add a comment about why we\u0027re not locking here? If you had to re-look this up I think it\u0027s best to have a comment so we don\u0027t forget about this a year or two from now.","commit_id":"5e5eedcd4f8f3b9969059a45a08a787284f658c8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b1b21e4749dee070b4b96b44a86c532c218fd237","unresolved":false,"context_lines":[{"line_number":9660,"context_line":"                fileutils.ensure_tree(cache_dir)"},{"line_number":9661,"context_line":"            LOG.info(\u0027Caching image %(image_id)s by request\u0027,"},{"line_number":9662,"context_line":"                     {\u0027image_id\u0027: image_id})"},{"line_number":9663,"context_line":"            images.fetch_to_raw(context, image_id, path)"},{"line_number":9664,"context_line":"            return True"},{"line_number":9665,"context_line":""},{"line_number":9666,"context_line":"    def _is_storage_shared_with(self, dest, inst_base):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_c17e1318","line":9663,"in_reply_to":"3fa7e38b_41d30361","updated":"2019-10-10 15:59:27.000000000","message":"Alright.","commit_id":"5e5eedcd4f8f3b9969059a45a08a787284f658c8"}]}