)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"938719eb593339a3fb7648c6ad6c1786dbf0e99c","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2018-05-10 16:45:51 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Cloning image fails results duplicate cache entry"},{"line_number":8,"context_line":"When creating a volume with image whose cache"},{"line_number":9,"context_line":"entry is already present,results in duplicate"},{"line_number":10,"context_line":"cache entry when create_clone_image() fails."},{"line_number":11,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"5f7c97a3_0e6a1aed","line":8,"updated":"2018-05-10 12:49:06.000000000","message":"You need a blank line after the summary line.","commit_id":"8d622e68d72f542b2a39660b5508cc816a265998"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"f5df4a2de018e54768a258875ff353ea3c489c06","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Vishakha Agarwal \u003cagarwalvishakha18@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-06-01 11:36:40 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Cloning image fails results duplicate cache entry"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When creating a volume with image whose cache"},{"line_number":10,"context_line":"entry is already present,results in duplicate"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"5f7c97a3_aceda58f","line":7,"range":{"start_line":7,"start_character":14,"end_line":7,"end_character":28},"updated":"2018-06-04 16:49:07.000000000","message":"failure results in","commit_id":"f40ca9a193197949360cc4e32ed36aca3c4b6595"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"f5df4a2de018e54768a258875ff353ea3c489c06","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Cloning image fails results duplicate cache entry"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When creating a volume with image whose cache"},{"line_number":10,"context_line":"entry is already present,results in duplicate"},{"line_number":11,"context_line":"cache entry when create_clone_image() fails."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: #1552734"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"5f7c97a3_6ce78dae","line":10,"range":{"start_line":10,"start_character":25,"end_line":10,"end_character":32},"updated":"2018-06-04 16:49:07.000000000","message":"missing space before \u0027results\u0027","commit_id":"f40ca9a193197949360cc4e32ed36aca3c4b6595"}],"cinder/image/cache.py":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"09fb7aba712350beb769635c218fdd8ce510c370","unresolved":false,"context_lines":[{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        return image_updated_utc !\u003d cache_updated_utc"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    def delete_duplicate_cache_entry(self, context, volume_ref, image_id):"},{"line_number":230,"context_line":"        # delete duplicate cache entry for cache"},{"line_number":231,"context_line":"        cache_entry \u003d self.db.image_volume_cache_get_and_update_last_used("},{"line_number":232,"context_line":"            context,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_8986ceb8","line":229,"updated":"2018-05-15 13:28:28.000000000","message":"I don\u0027t like this function name because there\u0027s nothing in the code that relates to the entry being a duplicate. Sure, the caller may feel the entry they\u0027re deleting is a duplicate, but this function simply deletes any entry it can find, regardless of whether it\u0027s actually a duplicate.\n\nI also don\u0027t think this function is necessary, because it\u0027s possible to prevent duplicate entries (see other comments).","commit_id":"2d6e6af873b74466e75807c02980037941e88162"}],"cinder/tests/unit/volume/test_image.py":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"b081ca6e473becd3579977e74e5fc966502f7d8e","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        self.mock_cache.get_entry.return_value \u003d mock_cache_entry"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        if mock_cache_entry:"},{"line_number":345,"context_line":"            # Entry is in cache, so basically don\u0027t do anything."},{"line_number":346,"context_line":"            # Make sure we didn\u0027t try and create a cache entry"},{"line_number":347,"context_line":"            self.assertFalse(self.mock_cache.ensure_space.called)"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f7c97a3_7af6ed91","line":344,"updated":"2018-06-08 20:42:37.000000000","message":"Are both paths for this conditional exercised? This test seems like a good candidate for using DDT input.\n\nSorry, I was traveling and unable to follow the latest patchsets for this review. If the unit test coverage can be improved then maybe we can follow up with another patch.","commit_id":"a74f7f0cd00619a775483f66f134ca8952eca7d3"},{"author":{"_account_id":27621,"name":"Vishakha Agarwal","email":"agarwalvishakha18@gmail.com","username":"Vishakha"},"change_message_id":"4a4670054a8c669a54e463315953d86a607063fe","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        self.mock_cache.get_entry.return_value \u003d mock_cache_entry"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        if mock_cache_entry:"},{"line_number":345,"context_line":"            # Entry is in cache, so basically don\u0027t do anything."},{"line_number":346,"context_line":"            # Make sure we didn\u0027t try and create a cache entry"},{"line_number":347,"context_line":"            self.assertFalse(self.mock_cache.ensure_space.called)"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f7c97a3_75310ded","line":344,"in_reply_to":"5f7c97a3_7af6ed91","updated":"2018-06-11 04:43:55.000000000","message":"Hi Abishop. Surely I will improve it.","commit_id":"a74f7f0cd00619a775483f66f134ca8952eca7d3"}],"cinder/tests/unit/volume/test_volume.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"938719eb593339a3fb7648c6ad6c1786dbf0e99c","unresolved":false,"context_lines":[{"line_number":2630,"context_line":"                          volume_dst)"},{"line_number":2631,"context_line":"        # Source volume\u0027s status and dst is set to available"},{"line_number":2632,"context_line":"        self.assertEqual(\u0027available\u0027, volume_src.status)"},{"line_number":2633,"context_line":"        self.assertEqual(\u0027available\u0027, volume_dst.status)"},{"line_number":2634,"context_line":"        self.volume.delete_volume(self.context, volume_dst)"},{"line_number":2635,"context_line":"        self.volume.delete_volume(self.context, volume_src)"},{"line_number":2636,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_ae43e66b","line":2633,"updated":"2018-05-10 12:49:06.000000000","message":"This doesn\u0027t seem right that volume_dst would be available after failure. Shouldn\u0027t that still be error?","commit_id":"8d622e68d72f542b2a39660b5508cc816a265998"}],"cinder/volume/flows/manager/create_volume.py":[{"author":{"_account_id":24952,"name":"Vivek Soni","email":"viveksoni31390@gmail.com","username":"sonivi"},"change_message_id":"cac13abedbb4caa9b2ef0db6644f8feeb00030e1","unresolved":false,"context_lines":[{"line_number":489,"context_line":"        # that use the source volume are underway."},{"line_number":490,"context_line":"        srcvol_ref \u003d objects.Volume.get_by_id(context, source_volid)"},{"line_number":491,"context_line":"        try:"},{"line_number":492,"context_line":"            model_update \u003d self.driver.create_cloned_volume(volume, srcvol_ref)"},{"line_number":493,"context_line":"        except exception.CinderException as e:"},{"line_number":494,"context_line":"            LOG.warning(\u0027Failed to clone volume.Error: %(exception)s\u0027,"},{"line_number":495,"context_line":"                        {\u0027exception\u0027: e})"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_74112652","line":492,"range":{"start_line":492,"start_character":12,"end_line":492,"end_character":24},"updated":"2018-05-10 09:13:59.000000000","message":"\"model_update\" will not be defined in case of failure in \"create_cloned_volume\" and thus failure here,\nhttp://logs.openstack.org/73/567473/4/check/openstack-tox-py27/a814d99/testr_results.html.gz\n\ncan we declare \"model_update\" as None at start of definition ?","commit_id":"1c4677d4adda22c42395a7b3af2278fcbd4ea27f"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"938719eb593339a3fb7648c6ad6c1786dbf0e99c","unresolved":false,"context_lines":[{"line_number":492,"context_line":"        try:"},{"line_number":493,"context_line":"            model_update \u003d self.driver.create_cloned_volume(volume, srcvol_ref)"},{"line_number":494,"context_line":"        except exception.CinderException as e:"},{"line_number":495,"context_line":"            LOG.warning(\u0027Failed to clone volume.Error: %(exception)s\u0027,"},{"line_number":496,"context_line":"                        {\u0027exception\u0027: e})"},{"line_number":497,"context_line":"            if volume:"},{"line_number":498,"context_line":"                self.manager.delete_volume(context, volume)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_2e7ef6ae","line":495,"range":{"start_line":495,"start_character":47,"end_line":495,"end_character":48},"updated":"2018-05-10 12:49:06.000000000","message":"Need space after period.","commit_id":"8d622e68d72f542b2a39660b5508cc816a265998"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"09fb7aba712350beb769635c218fdd8ce510c370","unresolved":false,"context_lines":[{"line_number":494,"context_line":"        except exception.CinderException as e:"},{"line_number":495,"context_line":"            LOG.warning(\u0027Failed to clone volume.Error: %(exception)s\u0027,"},{"line_number":496,"context_line":"                        {\u0027exception\u0027: e})"},{"line_number":497,"context_line":"            # will delete old cache entry"},{"line_number":498,"context_line":"            if self.image_volume_cache:"},{"line_number":499,"context_line":"                self.image_volume_cache.delete_duplicate_cache_entry("},{"line_number":500,"context_line":"                    context, volume, image_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_a912aa60","line":497,"updated":"2018-05-15 13:28:28.000000000","message":"This is not a good place to solve the problem. This function has no inherent knowledge of, or interest in the cache. The function is called from multiple places, and not all of them have anything to do with the cache. Basically, it\u0027s not safe for this function to assume a duplicate cache entry was created simply because create_cloned_volume() failed.","commit_id":"2d6e6af873b74466e75807c02980037941e88162"},{"author":{"_account_id":25376,"name":"zhuangzejian","email":"zzj123389602@gmail.com","username":"smartu3"},"change_message_id":"fca677fa529bfe49ebbe9e165527a4f8737e1b1c","unresolved":false,"context_lines":[{"line_number":495,"context_line":"            LOG.warning(\u0027Failed to clone volume.Error: %(exception)s\u0027,"},{"line_number":496,"context_line":"                        {\u0027exception\u0027: e})"},{"line_number":497,"context_line":"            # will delete old cache entry"},{"line_number":498,"context_line":"            if self.image_volume_cache:"},{"line_number":499,"context_line":"                self.image_volume_cache.delete_duplicate_cache_entry("},{"line_number":500,"context_line":"                    context, volume, image_id)"},{"line_number":501,"context_line":"        finally:"},{"line_number":502,"context_line":"            self._cleanup_cg_in_volume(volume)"},{"line_number":503,"context_line":"        # NOTE(harlowja): Subtasks would be useful here since after this"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_d899a8d0","line":500,"range":{"start_line":498,"start_character":0,"end_line":500,"end_character":46},"updated":"2018-05-18 11:14:09.000000000","message":"I read the Bug description again.\nThe reporter said that when create_cloned_volume failed, it will be a new image cache and the old image is still?\nMaybe we should focus on why this scene comes out and fixed.\nThe delete_duplicate_cache_entry will delete a available cache entry no matter what the driver has problem or the image cache is \u0027useless\u0027 in a way.","commit_id":"2d6e6af873b74466e75807c02980037941e88162"}],"cinder/volume/manager.py":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"54e5a59eba8000a6241888a8ccc07cabc4d09d70","unresolved":false,"context_lines":[{"line_number":1394,"context_line":"                                                                image_id,"},{"line_number":1395,"context_line":"                                                                image_meta)"},{"line_number":1396,"context_line":""},{"line_number":1397,"context_line":"            except NotImplementedError:"},{"line_number":1398,"context_line":"                LOG.warning(\u0027Cache entry does not exist with the image id :\u0027"},{"line_number":1399,"context_line":"                            \u0027%(cache_entry)s.Making new cache entry\u0027,"},{"line_number":1400,"context_line":"                            {\u0027cache_entry\u0027: cache_entry})"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f7c97a3_92ecf1f4","line":1397,"updated":"2018-05-29 11:57:22.000000000","message":"Please review the get_entry() code. It returns None when there is no entry, and does not throw an exception.","commit_id":"b78057db657570c877e1de0ef005af7b7728e0a6"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"54e5a59eba8000a6241888a8ccc07cabc4d09d70","unresolved":false,"context_lines":[{"line_number":1395,"context_line":"                                                                image_meta)"},{"line_number":1396,"context_line":""},{"line_number":1397,"context_line":"            except NotImplementedError:"},{"line_number":1398,"context_line":"                LOG.warning(\u0027Cache entry does not exist with the image id :\u0027"},{"line_number":1399,"context_line":"                            \u0027%(cache_entry)s.Making new cache entry\u0027,"},{"line_number":1400,"context_line":"                            {\u0027cache_entry\u0027: cache_entry})"},{"line_number":1401,"context_line":"                self.image_volume_cache.create_cache_entry("}],"source_content_type":"text/x-python","patch_set":8,"id":"5f7c97a3_f29f654d","line":1398,"updated":"2018-05-29 11:57:22.000000000","message":"This is not a good place to issue a WARNING message, although a DEBUG message is appropriate.\n\nI also think the logic can be simplified by using a pattern like you see starting on L1386.\n\ncache_entry \u003d self.image_volume_cache.get_entry()\nif cache_entry:\n    LOG.debug()\n    return","commit_id":"b78057db657570c877e1de0ef005af7b7728e0a6"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"03243dcd37264e394bd36b180c592939d2d5fc8b","unresolved":false,"context_lines":[{"line_number":1393,"context_line":"                                                                image_volume,"},{"line_number":1394,"context_line":"                                                                image_id,"},{"line_number":1395,"context_line":"                                                                image_meta)"},{"line_number":1396,"context_line":"            except NotImplementedError:"},{"line_number":1397,"context_line":"                LOG.debug(\u0027Cache entry does not exist with the image id :\u0027"},{"line_number":1398,"context_line":"                          \u0027%(image_id)s.Making new cache entry\u0027,"},{"line_number":1399,"context_line":"                          {\u0027image_id\u0027: image_id})"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_1c2d39fd","line":1396,"updated":"2018-05-30 17:00:53.000000000","message":"I\u0027m still not sure why you handle this exception. Is it raised when you call get_entry() and no entry exists?","commit_id":"64c9d52182278d15cfca52c5902f139500c546c3"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"03243dcd37264e394bd36b180c592939d2d5fc8b","unresolved":false,"context_lines":[{"line_number":1394,"context_line":"                                                                image_id,"},{"line_number":1395,"context_line":"                                                                image_meta)"},{"line_number":1396,"context_line":"            except NotImplementedError:"},{"line_number":1397,"context_line":"                LOG.debug(\u0027Cache entry does not exist with the image id :\u0027"},{"line_number":1398,"context_line":"                          \u0027%(image_id)s.Making new cache entry\u0027,"},{"line_number":1399,"context_line":"                          {\u0027image_id\u0027: image_id})"},{"line_number":1400,"context_line":"            if not cache_entry:"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_5c1f31b6","line":1397,"updated":"2018-05-30 17:00:53.000000000","message":"OK, but I was thinking a log message would be meaningful when a cache entry already exists. In other words, add a log message when this function doesn\u0027t need to create a new entry because an entry already exists.","commit_id":"64c9d52182278d15cfca52c5902f139500c546c3"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"03243dcd37264e394bd36b180c592939d2d5fc8b","unresolved":false,"context_lines":[{"line_number":1397,"context_line":"                LOG.debug(\u0027Cache entry does not exist with the image id :\u0027"},{"line_number":1398,"context_line":"                          \u0027%(image_id)s.Making new cache entry\u0027,"},{"line_number":1399,"context_line":"                          {\u0027image_id\u0027: image_id})"},{"line_number":1400,"context_line":"            if not cache_entry:"},{"line_number":1401,"context_line":"                self.image_volume_cache.create_cache_entry("},{"line_number":1402,"context_line":"                    ctx,"},{"line_number":1403,"context_line":"                    image_volume,"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_dce7c1ab","line":1400,"updated":"2018-05-30 17:00:53.000000000","message":"The main problem is the check for whether a cache entry already exists needs to occur before L1383. The image_volume clone should only be created when you truly need to create a new cache entry. This code will create a clone regardless of whether a cache entry exists.","commit_id":"64c9d52182278d15cfca52c5902f139500c546c3"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"4474f05a9ce6d7128199c072665961c53236e2e7","unresolved":false,"context_lines":[{"line_number":1397,"context_line":"                LOG.debug(\u0027Cache entry does not exist with the image id :\u0027"},{"line_number":1398,"context_line":"                          \u0027%(image_id)s.Making new cache entry\u0027,"},{"line_number":1399,"context_line":"                          {\u0027image_id\u0027: image_id})"},{"line_number":1400,"context_line":"            if not cache_entry:"},{"line_number":1401,"context_line":"                self.image_volume_cache.create_cache_entry("},{"line_number":1402,"context_line":"                    ctx,"},{"line_number":1403,"context_line":"                    image_volume,"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_878a5261","line":1400,"in_reply_to":"5f7c97a3_dce7c1ab","updated":"2018-05-30 17:13:08.000000000","message":"Actually, the check for whether a cache entry already exists should probably be at the start of the function, above L1373. There\u0027s no need to ensure_space() unless you need to create a new entry.","commit_id":"64c9d52182278d15cfca52c5902f139500c546c3"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"da811bae9c4b7b3cf83313d868f5e3d5dd98eadd","unresolved":false,"context_lines":[{"line_number":1371,"context_line":"        in the volume described by the volume_ref."},{"line_number":1372,"context_line":"        \"\"\""},{"line_number":1373,"context_line":"        image_volume \u003d None"},{"line_number":1374,"context_line":"        cache_entry \u003d self.image_volume_cache.get_entry(ctx,"},{"line_number":1375,"context_line":"                                                        image_volume,"},{"line_number":1376,"context_line":"                                                        image_id,"},{"line_number":1377,"context_line":"                                                        image_meta)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_2a00d501","line":1374,"updated":"2018-05-31 12:06:31.000000000","message":"Move the check for an existing entry (L1374-L1381) above L1373. See next comment for why you don\u0027t need image_volume to check for an existing entry.","commit_id":"1885173734d7335d798952ea6d6cf3dd79958781"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"da811bae9c4b7b3cf83313d868f5e3d5dd98eadd","unresolved":false,"context_lines":[{"line_number":1372,"context_line":"        \"\"\""},{"line_number":1373,"context_line":"        image_volume \u003d None"},{"line_number":1374,"context_line":"        cache_entry \u003d self.image_volume_cache.get_entry(ctx,"},{"line_number":1375,"context_line":"                                                        image_volume,"},{"line_number":1376,"context_line":"                                                        image_id,"},{"line_number":1377,"context_line":"                                                        image_meta)"},{"line_number":1378,"context_line":"        if cache_entry:"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_f52c029a","line":1375,"updated":"2018-05-31 12:06:31.000000000","message":"-1: No, you cannot use image_volume (which is None), use volume_ref instead.\n\nLook at the get_entry() code and you\u0027ll see how it uses this parameter, and why None will not work.\n\nAlso note that get_entry() names this parameter \"volume_ref\" ;-)","commit_id":"1885173734d7335d798952ea6d6cf3dd79958781"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"da811bae9c4b7b3cf83313d868f5e3d5dd98eadd","unresolved":false,"context_lines":[{"line_number":1379,"context_line":"            LOG.debug(\u0027 Cache exists with %(image_id)s\u0027,"},{"line_number":1380,"context_line":"                      {\u0027image_id\u0027: image_id})"},{"line_number":1381,"context_line":"            return"},{"line_number":1382,"context_line":"        else:"},{"line_number":1383,"context_line":"            try:"},{"line_number":1384,"context_line":"                if not self.image_volume_cache.ensure_space(ctx, volume_ref):"},{"line_number":1385,"context_line":"                    LOG.warning(\u0027Unable to ensure space for image-volume in\u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_6a0a4d26","line":1382,"updated":"2018-05-31 12:06:31.000000000","message":"You can eliminate the \u0027else\u0027 because the \u0027if\u0027 portion always returns. This will allow the original code to remain intact because the only thing this patch does is add a few lines (L1374-L1381, plus maybe a blank line after L1381).","commit_id":"1885173734d7335d798952ea6d6cf3dd79958781"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"b465096a3ad6dec895ea4d6c45b262f93a584508","unresolved":false,"context_lines":[{"line_number":1388,"context_line":"                            \u0027%(image_id)s will not create cache entry.\u0027,"},{"line_number":1389,"context_line":"                            {\u0027image_id\u0027: image_id})"},{"line_number":1390,"context_line":"                return"},{"line_number":1391,"context_line":""},{"line_number":1392,"context_line":"            self.image_volume_cache.create_cache_entry("},{"line_number":1393,"context_line":"                ctx,"},{"line_number":1394,"context_line":"                image_volume,"}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_8edd6642","side":"PARENT","line":1391,"updated":"2018-06-01 12:59:55.000000000","message":"nit: this line should not have been deleted","commit_id":"d2b3e1011e238ce1c29157e0614a0416a30448a8"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"f5df4a2de018e54768a258875ff353ea3c489c06","unresolved":false,"context_lines":[{"line_number":1375,"context_line":"                                                        image_id,"},{"line_number":1376,"context_line":"                                                        image_meta)"},{"line_number":1377,"context_line":"        if cache_entry:"},{"line_number":1378,"context_line":"            LOG.debug(\u0027 Cache exists with %(image_id)s\u0027,"},{"line_number":1379,"context_line":"                      {\u0027image_id\u0027: image_id})"},{"line_number":1380,"context_line":"            return"},{"line_number":1381,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_2cd8b5ee","line":1378,"range":{"start_line":1378,"start_character":23,"end_line":1378,"end_character":54},"updated":"2018-06-04 16:49:07.000000000","message":"In addition to Alan\u0027s comment I think this should say \u0027Cache entry already exists with %(image_id)s\u0027","commit_id":"f40ca9a193197949360cc4e32ed36aca3c4b6595"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"b465096a3ad6dec895ea4d6c45b262f93a584508","unresolved":false,"context_lines":[{"line_number":1375,"context_line":"                                                        image_id,"},{"line_number":1376,"context_line":"                                                        image_meta)"},{"line_number":1377,"context_line":"        if cache_entry:"},{"line_number":1378,"context_line":"            LOG.debug(\u0027 Cache exists with %(image_id)s\u0027,"},{"line_number":1379,"context_line":"                      {\u0027image_id\u0027: image_id})"},{"line_number":1380,"context_line":"            return"},{"line_number":1381,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_2ea452a5","line":1378,"updated":"2018-06-01 12:59:55.000000000","message":"nits:\n- Remove space before first word \"Cache\"\n- The message might seem vague when it appears in the log, mainly because it doesn\u0027t mention the image_id (the value) is an image ID. You\u0027ll see an entry that just says something like this:\n\n  \" Cache exists with 880a3ece-74b4-45aa-8e23-b3df089e6fd6\"\n\nI\u0027m not going to hold up the review because this is a DEBUG message.","commit_id":"f40ca9a193197949360cc4e32ed36aca3c4b6595"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"0feb5f4c8a6331b2ce3f91025743a9822c7ff010","unresolved":false,"context_lines":[{"line_number":1375,"context_line":"                                                        image_id,"},{"line_number":1376,"context_line":"                                                        image_meta)"},{"line_number":1377,"context_line":"        if cache_entry:"},{"line_number":1378,"context_line":"            LOG.debug(\u0027 Cache exists with %(image_id)s\u0027,"},{"line_number":1379,"context_line":"                      {\u0027image_id\u0027: image_id})"},{"line_number":1380,"context_line":"            return"},{"line_number":1381,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_f17dadb2","line":1378,"range":{"start_line":1378,"start_character":23,"end_line":1378,"end_character":54},"in_reply_to":"5f7c97a3_2cd8b5ee","updated":"2018-06-05 07:17:35.000000000","message":"\u0027Cache entry already exists with image ID %(image_id)s\u0027","commit_id":"f40ca9a193197949360cc4e32ed36aca3c4b6595"}]}