)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"6d71fce001257c702c02a39fd5e2d8b3f28b9ab0","unresolved":false,"context_lines":[{"line_number":20,"context_line":"specific configurables. nova.storage.rbd_utils.RBDDriver has also been"},{"line_number":21,"context_line":"modified to accept these but continues to default to the ``[libvirt]``"},{"line_number":22,"context_line":"specific configurables for now."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Change-Id: I3032bbe6bd2d6acc9ba0f0cac4d00ed4b4464ceb"},{"line_number":25,"context_line":"Implements: blueprint nova-image-download-via-rbd"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":40,"id":"9f560f44_a9ad1632","line":23,"updated":"2020-08-27 14:49:14.000000000","message":"Could we add a note about the un-deprecation here?","commit_id":"3253c743cd487b443aac0572a65b9b4979a53158"}],"nova/conf/glance.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f960f28dacbe5127e8046107cb37a9268a0de5b8","unresolved":false,"context_lines":[{"line_number":154,"context_line":"The RADOS client name for accessing Glance images stored as rbd volumes."},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"Related options:"},{"line_number":157,"context_line":"* This option is only used if allowed_direct_url_schemes contains `rbd`."},{"line_number":158,"context_line":"\"\"\"),"},{"line_number":159,"context_line":"    cfg.IntOpt(\u0027rbd_connect_timeout\u0027,"},{"line_number":160,"context_line":"               default\u003d5,"}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_e2b6333e","line":157,"range":{"start_line":157,"start_character":0,"end_line":157,"end_character":72},"updated":"2020-08-25 15:34:43.000000000","message":"This needs a newline before it to render properly in rST [1]\n\n[1] http://rst.ninjs.org/#VGVzdAoqIEhlbGxv","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"18ae6fd4b9b96645f053688e07921681ae1e9f26","unresolved":false,"context_lines":[{"line_number":154,"context_line":"The RADOS client name for accessing Glance images stored as rbd volumes."},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"Related options:"},{"line_number":157,"context_line":"* This option is only used if allowed_direct_url_schemes contains `rbd`."},{"line_number":158,"context_line":"\"\"\"),"},{"line_number":159,"context_line":"    cfg.IntOpt(\u0027rbd_connect_timeout\u0027,"},{"line_number":160,"context_line":"               default\u003d5,"}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_22c94b25","line":157,"range":{"start_line":157,"start_character":0,"end_line":157,"end_character":72},"in_reply_to":"9f560f44_e2b6333e","updated":"2020-08-25 15:56:33.000000000","message":"done","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"59c3fb1e0cf0198cab889937229c5cec4b7d790f","unresolved":false,"context_lines":[{"line_number":149,"context_line":"  enable_certificate_validation are enabled."},{"line_number":150,"context_line":"\"\"\"),"},{"line_number":151,"context_line":"    cfg.StrOpt(\u0027rbd_user\u0027,"},{"line_number":152,"context_line":"               default\u003d\u0027\u0027,"},{"line_number":153,"context_line":"               help\u003d\"\"\""},{"line_number":154,"context_line":"The RADOS client name for accessing Glance images stored as rbd volumes."},{"line_number":155,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"9f560f44_f1f53a52","line":152,"range":{"start_line":152,"start_character":8,"end_line":152,"end_character":15},"updated":"2020-08-27 10:10:20.000000000","message":"Also, missed this in the earlier PS, but this should go. It\u0027s not present above","commit_id":"8e408cbdc45a6b6e12d5e878d00eae0d9c62365a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4cc0ab9266996b545478abf4a5ba816638182478","unresolved":false,"context_lines":[{"line_number":154,"context_line":"The RADOS client name for accessing Glance images stored as rbd volumes."},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"Related options:"},{"line_number":157,"context_line":"* This option is only used if allowed_direct_url_schemes contains `rbd`."},{"line_number":158,"context_line":"\"\"\"),"},{"line_number":159,"context_line":"    cfg.IntOpt(\u0027rbd_connect_timeout\u0027,"},{"line_number":160,"context_line":"               default\u003d5,"}],"source_content_type":"text/x-python","patch_set":39,"id":"9f560f44_f1409aa4","line":157,"updated":"2020-08-27 10:08:09.000000000","message":"This is still wrong","commit_id":"8e408cbdc45a6b6e12d5e878d00eae0d9c62365a"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"6d71fce001257c702c02a39fd5e2d8b3f28b9ab0","unresolved":false,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"Related options:"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"* This option is only used if allowed_direct_url_schemes contains `rbd`."},{"line_number":159,"context_line":"\"\"\"),"},{"line_number":160,"context_line":"    cfg.IntOpt(\u0027rbd_connect_timeout\u0027,"},{"line_number":161,"context_line":"        default\u003d5,"}],"source_content_type":"text/x-python","patch_set":40,"id":"9f560f44_c9f96ac8","line":158,"range":{"start_line":158,"start_character":30,"end_line":158,"end_character":56},"updated":"2020-08-27 14:49:14.000000000","message":"That\u0027s listed as being deprecated for removal, is that still true with this patch?\n\n\u003clater\u003e Oh, https://review.opendev.org/#/c/728095/11 is a thing. Looks like the series needs to be rebased for patches to show up as being properly on top of each other.","commit_id":"3253c743cd487b443aac0572a65b9b4979a53158"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"ba08433fad04fde300f00cc8f0ef705a65cbe057","unresolved":false,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"Related options:"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"* This option is only used if allowed_direct_url_schemes contains `rbd`."},{"line_number":159,"context_line":"\"\"\"),"},{"line_number":160,"context_line":"    cfg.IntOpt(\u0027rbd_connect_timeout\u0027,"},{"line_number":161,"context_line":"        default\u003d5,"}],"source_content_type":"text/x-python","patch_set":40,"id":"9f560f44_5c5ded46","line":158,"range":{"start_line":158,"start_character":30,"end_line":158,"end_character":56},"in_reply_to":"9f560f44_2ad8c21c","updated":"2020-08-31 09:56:47.000000000","message":"Well, this would be changed with https://review.opendev.org/#/c/748411/ , right?","commit_id":"3253c743cd487b443aac0572a65b9b4979a53158"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"40808181fccc9429a2dbd924601290f4214d1585","unresolved":false,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"Related options:"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"* This option is only used if allowed_direct_url_schemes contains `rbd`."},{"line_number":159,"context_line":"\"\"\"),"},{"line_number":160,"context_line":"    cfg.IntOpt(\u0027rbd_connect_timeout\u0027,"},{"line_number":161,"context_line":"        default\u003d5,"}],"source_content_type":"text/x-python","patch_set":40,"id":"9f560f44_87e1f22d","line":158,"range":{"start_line":158,"start_character":30,"end_line":158,"end_character":56},"in_reply_to":"9f560f44_5c5ded46","updated":"2020-08-31 10:14:08.000000000","message":"It would be better not to flip-flop in code. Feel free to merge [1] into this patch. You don\u0027t need to credit me as it\u0027s very trivial.\n\n[1] https://review.opendev.org/#/c/748411/","commit_id":"3253c743cd487b443aac0572a65b9b4979a53158"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"37270038f35340826cadcffba573b90d1c291d59","unresolved":false,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"Related options:"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"* This option is only used if allowed_direct_url_schemes contains `rbd`."},{"line_number":159,"context_line":"\"\"\"),"},{"line_number":160,"context_line":"    cfg.IntOpt(\u0027rbd_connect_timeout\u0027,"},{"line_number":161,"context_line":"        default\u003d5,"}],"source_content_type":"text/x-python","patch_set":40,"id":"9f560f44_2ad8c21c","line":158,"range":{"start_line":158,"start_character":30,"end_line":158,"end_character":56},"in_reply_to":"9f560f44_c9f96ac8","updated":"2020-08-28 15:28:13.000000000","message":"Wait, if we\u0027ve finally settled on [glance]/enable_rbd_download, this needs to be changed.","commit_id":"3253c743cd487b443aac0572a65b9b4979a53158"}],"nova/image/download/rbd.py":[{"author":{"_account_id":10338,"name":"Daniel Speichert","email":"daniel@speichert.pl","username":"daniel"},"change_message_id":"c9874d99e823937d4c7151a4e3efbc4e00be055a","unresolved":false,"context_lines":[{"line_number":12,"context_line":"# License for the specific language governing permissions and limitations"},{"line_number":13,"context_line":"# under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import logging"},{"line_number":16,"context_line":"import urllib"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import nova.conf"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_f5cc2f62","line":15,"updated":"2018-06-11 15:52:37.000000000","message":"from oslo_log import log as logging","commit_id":"e8c149a8b8fe57e954088d82583caafd53d2a0b2"},{"author":{"_account_id":10338,"name":"Daniel Speichert","email":"daniel@speichert.pl","username":"daniel"},"change_message_id":"c9874d99e823937d4c7151a4e3efbc4e00be055a","unresolved":false,"context_lines":[{"line_number":13,"context_line":"# under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import logging"},{"line_number":16,"context_line":"import urllib"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import nova.conf"},{"line_number":19,"context_line":"import nova.exception as exception"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_75e1ffcf","line":16,"updated":"2018-06-11 15:52:37.000000000","message":"from six.moves import urllib\n\nor are we not using six anymore?","commit_id":"e8c149a8b8fe57e954088d82583caafd53d2a0b2"},{"author":{"_account_id":10338,"name":"Daniel Speichert","email":"daniel@speichert.pl","username":"daniel"},"change_message_id":"c9874d99e823937d4c7151a4e3efbc4e00be055a","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"class RBDDownload(object):"},{"line_number":28,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":29,"context_line":"        self.rbd_driver \u003d None"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def download(self, context, url_parts, dst_path, metadata, **kwargs):"},{"line_number":32,"context_line":"        if self.rbd_driver is None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_355ce77b","line":29,"updated":"2018-06-11 15:52:37.000000000","message":"is this property used anywhere?","commit_id":"e8c149a8b8fe57e954088d82583caafd53d2a0b2"},{"author":{"_account_id":10338,"name":"Daniel Speichert","email":"daniel@speichert.pl","username":"daniel"},"change_message_id":"c9874d99e823937d4c7151a4e3efbc4e00be055a","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    def download(self, context, url_parts, dst_path, metadata, **kwargs):"},{"line_number":32,"context_line":"        if self.rbd_driver is None:"},{"line_number":33,"context_line":"            self.rbd_driver \u003d rbd_utils.RBDDriver("},{"line_number":34,"context_line":"                pool\u003dCONF.libvirt.images_rbd_pool,"},{"line_number":35,"context_line":"                ceph_conf\u003dCONF.libvirt.images_rbd_ceph_conf,"},{"line_number":36,"context_line":"                rbd_user\u003dCONF.libvirt.rbd_user)"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_954a9baa","line":34,"range":{"start_line":34,"start_character":16,"end_line":34,"end_character":20},"updated":"2018-06-11 15:52:37.000000000","message":"you can assemble RBDDriver with the correct pool panem after getting it from the parsed URL","commit_id":"e8c149a8b8fe57e954088d82583caafd53d2a0b2"},{"author":{"_account_id":10338,"name":"Daniel Speichert","email":"daniel@speichert.pl","username":"daniel"},"change_message_id":"c9874d99e823937d4c7151a4e3efbc4e00be055a","unresolved":false,"context_lines":[{"line_number":40,"context_line":"            # sections and be in the format of:"},{"line_number":41,"context_line":"            # \u003ccluster_uuid\u003e/\u003cpool_name\u003e/\u003cimage_uuid\u003e/\u003csnapshot_name\u003e"},{"line_number":42,"context_line":"            url_path \u003d str(urllib.unquote(url_parts.path))"},{"line_number":43,"context_line":"            cluster_uuid, pool_name, image_uuid, snapshot_name \u003d ("},{"line_number":44,"context_line":"                url_path.split(\u0027/\u0027))"},{"line_number":45,"context_line":"        except ValueError as e:"},{"line_number":46,"context_line":"            msg \u003d _(\"Invalid RBD URL format: %s\") % e"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_f523cf03","line":43,"updated":"2018-06-11 15:52:37.000000000","message":"fsid is not used anyway.\nRBDDriver.parse_url could parse the URL from urlparts.geturl().\n\nor: pool, image_name, snap \u003d [urllib.parse.unquote(piece) for piece in url_parts.path.lstrip(\u0027/\u0027).split(\u0027/\u0027)]","commit_id":"e8c149a8b8fe57e954088d82583caafd53d2a0b2"},{"author":{"_account_id":10338,"name":"Daniel Speichert","email":"daniel@speichert.pl","username":"daniel"},"change_message_id":"e614230dc6e9cf69d7ee4d5d2cdbdb297267d83b","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"class RBDDownload(object):"},{"line_number":28,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":29,"context_line":"        pass"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def download(self, context, url_parts, dst_path, metadata, **kwargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_a8f5fa52","line":28,"updated":"2018-06-11 16:28:27.000000000","message":"I don\u0027t see the point of having the constructor at all","commit_id":"96bfceda115d5f15229055271303f66ac20dbf50"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"bc8d638c9d647abfcac11b91d07e040e358f1871","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from six.moves import urllib"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import nova.conf"},{"line_number":19,"context_line":"import nova.exception as exception"},{"line_number":20,"context_line":"from nova.i18n import _"},{"line_number":21,"context_line":"from nova.virt.libvirt.storage import rbd_utils"},{"line_number":22,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1f493fa4_0c171ea6","line":19,"updated":"2020-04-29 14:11:23.000000000","message":"pep8: N359: Import alias should not be redundant.","commit_id":"4046906700f434b941e861558d9cfea156c10729"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"bc8d638c9d647abfcac11b91d07e040e358f1871","unresolved":false,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":"def get_schemes():"},{"line_number":72,"context_line":"    return [\u0027rbd\u0027]"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1f493fa4_ec1912d7","line":73,"updated":"2020-04-29 14:11:23.000000000","message":"pep8: W391 blank line at end of file","commit_id":"4046906700f434b941e861558d9cfea156c10729"}],"nova/image/glance.py":[{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"bc8d638c9d647abfcac11b91d07e040e358f1871","unresolved":false,"context_lines":[{"line_number":334,"context_line":"                        # Load chunks from the downloaded image file"},{"line_number":335,"context_line":"                        data \u003d open(dst_path, \u0027wb\u0027)"},{"line_number":336,"context_line":"                        content_length \u003d os.path.getsize(dst_path)"},{"line_number":337,"context_line":"                        image_chunks \u003d utils.IterableWithLength(data, content_length)"},{"line_number":338,"context_line":"                    except Exception:"},{"line_number":339,"context_line":"                        LOG.exception(\"Download image error\")"},{"line_number":340,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1f493fa4_2c101aa8","line":337,"updated":"2020-04-29 14:11:23.000000000","message":"pep8: E501 line too long (85 \u003e 79 characters)","commit_id":"4046906700f434b941e861558d9cfea156c10729"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"bc8d638c9d647abfcac11b91d07e040e358f1871","unresolved":false,"context_lines":[{"line_number":346,"context_line":"                _reraise_translated_image_exception(image_id)"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"            if image_chunks.wrapped is None:"},{"line_number":349,"context_line":"                # None is a valid return value, but there\u0027s nothing we can do with"},{"line_number":350,"context_line":"                # a image with no associated data"},{"line_number":351,"context_line":"                raise exception.ImageUnacceptable(image_id\u003dimage_id,"},{"line_number":352,"context_line":"                    reason\u003d\u0027Image has no associated data\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"1f493fa4_8c2a0ed8","line":349,"updated":"2020-04-29 14:11:23.000000000","message":"pep8: E501 line too long (82 \u003e 79 characters)","commit_id":"4046906700f434b941e861558d9cfea156c10729"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"18daaf679802cb9284fd2756e89b47a0a46baa5e","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            # Parse the RBD URL from url_parts, it should consist of 4"},{"line_number":233,"context_line":"            # sections and be in the format of:"},{"line_number":234,"context_line":"            # \u003ccluster_uuid\u003e/\u003cpool_name\u003e/\u003cimage_uuid\u003e/\u003csnapshot_name\u003e"},{"line_number":235,"context_line":"            url_path \u003d str(urllib.parse.unquote(url_parts.path))"},{"line_number":236,"context_line":"            cluster_uuid, pool_name, image_uuid, snapshot_name \u003d ("},{"line_number":237,"context_line":"                url_path.split(\u0027/\u0027))"},{"line_number":238,"context_line":"        except ValueError as e:"}],"source_content_type":"text/x-python","patch_set":11,"id":"1f493fa4_00230d07","line":235,"updated":"2020-05-04 20:11:32.000000000","message":"pep8: F821 undefined name \u0027urllib\u0027","commit_id":"b222844684f0f0cb58a958bee83ddf41d555150f"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"18daaf679802cb9284fd2756e89b47a0a46baa5e","unresolved":false,"context_lines":[{"line_number":236,"context_line":"            cluster_uuid, pool_name, image_uuid, snapshot_name \u003d ("},{"line_number":237,"context_line":"                url_path.split(\u0027/\u0027))"},{"line_number":238,"context_line":"        except ValueError as e:"},{"line_number":239,"context_line":"            msg \u003d _(\"Invalid RBD URL format: %s\") % e"},{"line_number":240,"context_line":"            LOG.error(msg)"},{"line_number":241,"context_line":"            raise nova.exception.InvalidParameterValue(reason\u003dmsg)"},{"line_number":242,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1f493fa4_602011f9","line":239,"updated":"2020-05-04 20:11:32.000000000","message":"pep8: N323: Found use of _() without explicit import of _ !","commit_id":"b222844684f0f0cb58a958bee83ddf41d555150f"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"18daaf679802cb9284fd2756e89b47a0a46baa5e","unresolved":false,"context_lines":[{"line_number":240,"context_line":"            LOG.error(msg)"},{"line_number":241,"context_line":"            raise nova.exception.InvalidParameterValue(reason\u003dmsg)"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver("},{"line_number":244,"context_line":"            pool\u003dpool_name,"},{"line_number":245,"context_line":"            ceph_conf\u003dCONF.libvirt.images_rbd_ceph_conf,"},{"line_number":246,"context_line":"            rbd_user\u003dCONF.libvirt.rbd_user)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1f493fa4_401d15bc","line":243,"updated":"2020-05-04 20:11:32.000000000","message":"pep8: F821 undefined name \u0027rbd_utils\u0027","commit_id":"b222844684f0f0cb58a958bee83ddf41d555150f"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"18daaf679802cb9284fd2756e89b47a0a46baa5e","unresolved":false,"context_lines":[{"line_number":246,"context_line":"            rbd_user\u003dCONF.libvirt.rbd_user)"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"        try:"},{"line_number":249,"context_line":"            msg \u003d _("},{"line_number":250,"context_line":"                \"Attempting to export RBD image: \""},{"line_number":251,"context_line":"                \"[pool_name: %s] \""},{"line_number":252,"context_line":"                \"[image_uuid: %s] \""}],"source_content_type":"text/x-python","patch_set":11,"id":"1f493fa4_a077b908","line":249,"updated":"2020-05-04 20:11:32.000000000","message":"pep8: N323: Found use of _() without explicit import of _ !","commit_id":"b222844684f0f0cb58a958bee83ddf41d555150f"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"18daaf679802cb9284fd2756e89b47a0a46baa5e","unresolved":false,"context_lines":[{"line_number":261,"context_line":"                snapshot_name,"},{"line_number":262,"context_line":"                pool_name)"},{"line_number":263,"context_line":"        except Exception as e:"},{"line_number":264,"context_line":"            msg \u003d _(\"Error during RBD image export: %s\") % e"},{"line_number":265,"context_line":"            LOG.error(msg)"},{"line_number":266,"context_line":"            raise nova.exception.CouldNotFetchImage(image_id\u003dimage_uuid)"},{"line_number":267,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1f493fa4_807c3dec","line":264,"updated":"2020-05-04 20:11:32.000000000","message":"pep8: N323: Found use of _() without explicit import of _ !","commit_id":"b222844684f0f0cb58a958bee83ddf41d555150f"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"e59b84f87e33a65ffcd948b5ea9f102aa34ea785","unresolved":false,"context_lines":[{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    def rbd_download(self, context, url_parts, dst_path, metadata):"},{"line_number":232,"context_line":"        # NOTE(jsuchome): It is pretty ugly to import the module here,"},{"line_number":233,"context_line":"        # however if we\u0027d have it on the usual place, it would bring cyclic dependency"},{"line_number":234,"context_line":"        # which is not simple to break without larger refactoring."},{"line_number":235,"context_line":"        from nova.virt.libvirt.storage import rbd_utils"},{"line_number":236,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f493fa4_30875bda","line":233,"updated":"2020-05-05 21:08:14.000000000","message":"pep8: E501 line too long (86 \u003e 79 characters)","commit_id":"32d76d953e68e3ee10a41f3d19b8a5866cb893d8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3eb2424ea12d34f7101b4b4e6bef9c8401b79b27","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        if \u0027rbd\u0027 in CONF.glance.allowed_direct_url_schemes:"},{"line_number":229,"context_line":"            self._download_handlers[\u0027rbd\u0027] \u003d self.rbd_download"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    def rbd_download(self, context, url_parts, dst_path, metadata):"},{"line_number":232,"context_line":"        # NOTE(jsuchome): It is pretty ugly to import the module here,"},{"line_number":233,"context_line":"        # however if we\u0027d have it on the usual place, it would bring cyclic"},{"line_number":234,"context_line":"        # dependency which is not easy to break without larger refactoring."}],"source_content_type":"text/x-python","patch_set":15,"id":"ff570b3c_580e8b8d","line":231,"updated":"2020-05-12 14:42:09.000000000","message":"This whole thing is untested, AFAICT","commit_id":"2aca0016075309adc6d478c7f255661648b1756a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3eb2424ea12d34f7101b4b4e6bef9c8401b79b27","unresolved":false,"context_lines":[{"line_number":393,"context_line":"            data \u003d open(dst_path, \u0027wb\u0027)"},{"line_number":394,"context_line":"            close_file \u003d True"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        if data is None or downloaded_length:"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"            # Perform image signature verification"},{"line_number":399,"context_line":"            if verifier:"}],"source_content_type":"text/x-python","patch_set":15,"id":"ff570b3c_585ccb7d","line":396,"range":{"start_line":396,"start_character":24,"end_line":396,"end_character":44},"updated":"2020-05-12 14:42:09.000000000","message":"When could we hit this when data is not None? Unless I\u0027m missing something, this is just dead code that isn\u0027t tested anyway.","commit_id":"2aca0016075309adc6d478c7f255661648b1756a"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"c3622d8069a5b433e002fba42835f8afcd94010b","unresolved":false,"context_lines":[{"line_number":393,"context_line":"            data \u003d open(dst_path, \u0027wb\u0027)"},{"line_number":394,"context_line":"            close_file \u003d True"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        if data is None or downloaded_length:"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"            # Perform image signature verification"},{"line_number":399,"context_line":"            if verifier:"}],"source_content_type":"text/x-python","patch_set":15,"id":"ff570b3c_b3738815","line":396,"range":{"start_line":396,"start_character":24,"end_line":396,"end_character":44},"in_reply_to":"ff570b3c_585ccb7d","updated":"2020-05-12 15:22:27.000000000","message":"Well, when data is not None, we do this part because the image was downloaded via the rbd export...","commit_id":"2aca0016075309adc6d478c7f255661648b1756a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3eb2424ea12d34f7101b4b4e6bef9c8401b79b27","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":410,"context_line":"                        LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":411,"context_line":"                                  \u0027for image: %s\u0027, image_id)"},{"line_number":412,"context_line":"            if data is None:"},{"line_number":413,"context_line":"                return image_chunks"},{"line_number":414,"context_line":"        else:"},{"line_number":415,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":15,"id":"ff570b3c_189533e5","line":412,"updated":"2020-05-12 14:42:09.000000000","message":"Why? Won\u0027t this make everything that enters this currently fall through and not return image_chunks? This changes the path for everything *but* the rbd case, AFAICT.","commit_id":"2aca0016075309adc6d478c7f255661648b1756a"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"c3622d8069a5b433e002fba42835f8afcd94010b","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":410,"context_line":"                        LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":411,"context_line":"                                  \u0027for image: %s\u0027, image_id)"},{"line_number":412,"context_line":"            if data is None:"},{"line_number":413,"context_line":"                return image_chunks"},{"line_number":414,"context_line":"        else:"},{"line_number":415,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":15,"id":"ff570b3c_738570ad","line":412,"in_reply_to":"ff570b3c_189533e5","updated":"2020-05-12 15:22:27.000000000","message":"Nope. This (I admit, weird) case is there really to keep the original behavior for all but rbd case.\n\nFor rbd case, the data is NOT None, because we opened it right after downloading the image.\n\nTBH, I do not know why this method is returning something sometimes and in some other cases nothing. So maybe we could return image_chunks always, it\u0027s just that your observation is not correct here.","commit_id":"2aca0016075309adc6d478c7f255661648b1756a"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"19065505b7d8a102b63cf87a6692db4c77c6674d","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        if \u0027rbd\u0027 in CONF.glance.allowed_direct_url_schemes:"},{"line_number":229,"context_line":"            self._download_handlers[\u0027rbd\u0027] \u003d self.rbd_download"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    def rbd_download(self, context, url_parts, dst_path, metadata):"},{"line_number":232,"context_line":"        # NOTE(jsuchome): It is pretty ugly to import the module here,"},{"line_number":233,"context_line":"        # however if we\u0027d have it on the usual place, it would bring cyclic"},{"line_number":234,"context_line":"        # dependency which is not easy to break without larger refactoring."}],"source_content_type":"text/x-python","patch_set":16,"id":"ff570b3c_9628e486","line":231,"range":{"start_line":231,"start_character":27,"end_line":231,"end_character":65},"updated":"2020-05-13 01:35:34.000000000","message":"You should comment these parametes, and the exception.","commit_id":"c7088e9a44b868fa9700e7de1563d2ed873c92f1"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":234,"context_line":"        :param context: The `nova.context.RequestContext` object for the"},{"line_number":235,"context_line":"                        request"},{"line_number":236,"context_line":"        :param url_parts: Parts of URL pointing to the image location"},{"line_number":237,"context_line":"        :param dest_path: Filepath to transfer the image file to."},{"line_number":238,"context_line":"        :param metadata: Image location metadata (currently unused)"},{"line_number":239,"context_line":"        \"\"\""},{"line_number":240,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_f530baa1","line":237,"range":{"start_line":237,"start_character":15,"end_line":237,"end_character":24},"updated":"2020-05-19 15:29:10.000000000","message":"dst_path but dest_path makes more sense to me.","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5083c5a50c2111707b11940e660ef5ce92252bfe","unresolved":false,"context_lines":[{"line_number":234,"context_line":"        :param context: The `nova.context.RequestContext` object for the"},{"line_number":235,"context_line":"                        request"},{"line_number":236,"context_line":"        :param url_parts: Parts of URL pointing to the image location"},{"line_number":237,"context_line":"        :param dest_path: Filepath to transfer the image file to."},{"line_number":238,"context_line":"        :param metadata: Image location metadata (currently unused)"},{"line_number":239,"context_line":"        \"\"\""},{"line_number":240,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_751ad416","line":237,"range":{"start_line":237,"start_character":15,"end_line":237,"end_character":24},"in_reply_to":"ff570b3c_f530baa1","updated":"2020-05-20 09:45:48.000000000","message":"Hm, but dst_path is already used in the download method. Let\u0027s stick to this one then.","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":239,"context_line":"        \"\"\""},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        # NOTE(jsuchome): It is pretty ugly to import the module here,"},{"line_number":242,"context_line":"        # however if we\u0027d have it on the usual place, it would bring cyclic"},{"line_number":243,"context_line":"        # dependency which is not easy to break without larger refactoring."},{"line_number":244,"context_line":"        from nova.virt.libvirt.storage import rbd_utils"},{"line_number":245,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_95bfbea8","line":242,"range":{"start_line":242,"start_character":69,"end_line":242,"end_character":75},"updated":"2020-05-19 15:29:10.000000000","message":"circular?","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        :param metadata: Image location metadata (currently unused)"},{"line_number":239,"context_line":"        \"\"\""},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        # NOTE(jsuchome): It is pretty ugly to import the module here,"},{"line_number":242,"context_line":"        # however if we\u0027d have it on the usual place, it would bring cyclic"},{"line_number":243,"context_line":"        # dependency which is not easy to break without larger refactoring."},{"line_number":244,"context_line":"        from nova.virt.libvirt.storage import rbd_utils"},{"line_number":245,"context_line":"        try:"},{"line_number":246,"context_line":"            # Parse the RBD URL from url_parts, it should consist of 4"},{"line_number":247,"context_line":"            # sections and be in the format of:"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_b5c48241","line":244,"range":{"start_line":241,"start_character":0,"end_line":244,"end_character":55},"updated":"2020-05-19 15:29:10.000000000","message":"Could you note what the actual cause of the circular dependency is?","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":250,"context_line":"            cluster_uuid, pool_name, image_uuid, snapshot_name \u003d ("},{"line_number":251,"context_line":"                url_path.split(\u0027/\u0027))"},{"line_number":252,"context_line":"        except ValueError as e:"},{"line_number":253,"context_line":"            msg \u003d _(\"Invalid RBD URL format: %s\") % e"},{"line_number":254,"context_line":"            LOG.error(msg)"},{"line_number":255,"context_line":"            raise nova.exception.InvalidParameterValue(msg)"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver()"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_b59122f4","line":254,"range":{"start_line":253,"start_character":0,"end_line":254,"end_character":26},"updated":"2020-05-19 15:29:10.000000000","message":"nit - just fold this into a single line","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5083c5a50c2111707b11940e660ef5ce92252bfe","unresolved":false,"context_lines":[{"line_number":250,"context_line":"            cluster_uuid, pool_name, image_uuid, snapshot_name \u003d ("},{"line_number":251,"context_line":"                url_path.split(\u0027/\u0027))"},{"line_number":252,"context_line":"        except ValueError as e:"},{"line_number":253,"context_line":"            msg \u003d _(\"Invalid RBD URL format: %s\") % e"},{"line_number":254,"context_line":"            LOG.error(msg)"},{"line_number":255,"context_line":"            raise nova.exception.InvalidParameterValue(msg)"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver()"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_d59ac05f","line":254,"range":{"start_line":253,"start_character":0,"end_line":254,"end_character":26},"in_reply_to":"ff570b3c_b59122f4","updated":"2020-05-20 09:45:48.000000000","message":"Actually - the msg is used twice, as an exception parameter on next line...","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"4d4724f7c5288bc3732e242d0b41f81f0fcae90c","unresolved":false,"context_lines":[{"line_number":250,"context_line":"            cluster_uuid, pool_name, image_uuid, snapshot_name \u003d ("},{"line_number":251,"context_line":"                url_path.split(\u0027/\u0027))"},{"line_number":252,"context_line":"        except ValueError as e:"},{"line_number":253,"context_line":"            msg \u003d _(\"Invalid RBD URL format: %s\") % e"},{"line_number":254,"context_line":"            LOG.error(msg)"},{"line_number":255,"context_line":"            raise nova.exception.InvalidParameterValue(msg)"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver()"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_c7d85354","line":254,"range":{"start_line":253,"start_character":0,"end_line":254,"end_character":26},"in_reply_to":"ff570b3c_d59ac05f","updated":"2020-05-20 10:23:40.000000000","message":"Doh apologies.","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":257,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver()"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"        try:"},{"line_number":260,"context_line":"            msg \u003d _("},{"line_number":261,"context_line":"                \"Attempting to export RBD image: \""},{"line_number":262,"context_line":"                \"[pool_name: %s] \""},{"line_number":263,"context_line":"                \"[image_uuid: %s] \""}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_5571a675","line":260,"range":{"start_line":260,"start_character":18,"end_line":260,"end_character":19},"updated":"2020-05-19 15:29:10.000000000","message":"We don\u0027t translate DEBUG messages so this isn\u0027t required, you can just fold all of this into a single LOG.debug line.","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5083c5a50c2111707b11940e660ef5ce92252bfe","unresolved":false,"context_lines":[{"line_number":257,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver()"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"        try:"},{"line_number":260,"context_line":"            msg \u003d _("},{"line_number":261,"context_line":"                \"Attempting to export RBD image: \""},{"line_number":262,"context_line":"                \"[pool_name: %s] \""},{"line_number":263,"context_line":"                \"[image_uuid: %s] \""}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_d585a008","line":260,"range":{"start_line":260,"start_character":18,"end_line":260,"end_character":19},"in_reply_to":"ff570b3c_5571a675","updated":"2020-05-20 09:45:48.000000000","message":"OK. I guess we don\u0027t have to translate ERROR messages either and remove import _ (which was brought in with this patch only)","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def download(self, context, image_id, data\u003dNone, dst_path\u003dNone,"},{"line_number":355,"context_line":"                 trusted_certs\u003dNone):"},{"line_number":356,"context_line":"        \"\"\"Calls out to Glance for data and writes data.\"\"\""},{"line_number":357,"context_line":"        image_chunks \u003d None"},{"line_number":358,"context_line":"        downloaded_length \u003d 0"},{"line_number":359,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_98ed35b9","line":356,"range":{"start_line":356,"start_character":8,"end_line":356,"end_character":59},"updated":"2020-05-19 15:29:10.000000000","message":"docstrings would be super useful here.","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":355,"context_line":"                 trusted_certs\u003dNone):"},{"line_number":356,"context_line":"        \"\"\"Calls out to Glance for data and writes data.\"\"\""},{"line_number":357,"context_line":"        image_chunks \u003d None"},{"line_number":358,"context_line":"        downloaded_length \u003d 0"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        if CONF.glance.allowed_direct_url_schemes and dst_path is not None:"},{"line_number":361,"context_line":"            image \u003d self.show(context, image_id, include_locations\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_fb4f0b68","line":358,"range":{"start_line":358,"start_character":8,"end_line":358,"end_character":29},"updated":"2020-05-19 15:29:10.000000000","message":"I think we can drop this.","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5083c5a50c2111707b11940e660ef5ce92252bfe","unresolved":false,"context_lines":[{"line_number":355,"context_line":"                 trusted_certs\u003dNone):"},{"line_number":356,"context_line":"        \"\"\"Calls out to Glance for data and writes data.\"\"\""},{"line_number":357,"context_line":"        image_chunks \u003d None"},{"line_number":358,"context_line":"        downloaded_length \u003d 0"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        if CONF.glance.allowed_direct_url_schemes and dst_path is not None:"},{"line_number":361,"context_line":"            image \u003d self.show(context, image_id, include_locations\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_3527bccc","line":358,"range":{"start_line":358,"start_character":8,"end_line":358,"end_character":29},"in_reply_to":"ff570b3c_fb4f0b68","updated":"2020-05-20 09:45:48.000000000","message":"I don\u0027t think so, see bellow","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":371,"context_line":""},{"line_number":372,"context_line":"                        # Load chunks from the downloaded image file"},{"line_number":373,"context_line":"                        # for verification (if required)"},{"line_number":374,"context_line":"                        data \u003d open(dst_path, \u0027rb\u0027)"},{"line_number":375,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":376,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":377,"context_line":"                            downloaded_length)"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_1b247fae","line":374,"range":{"start_line":374,"start_character":0,"end_line":374,"end_character":51},"updated":"2020-05-19 15:29:10.000000000","message":"I think we can remove this and use L397-400 to open the file.","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5083c5a50c2111707b11940e660ef5ce92252bfe","unresolved":false,"context_lines":[{"line_number":371,"context_line":""},{"line_number":372,"context_line":"                        # Load chunks from the downloaded image file"},{"line_number":373,"context_line":"                        # for verification (if required)"},{"line_number":374,"context_line":"                        data \u003d open(dst_path, \u0027rb\u0027)"},{"line_number":375,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":376,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":377,"context_line":"                            downloaded_length)"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_15cef858","line":374,"range":{"start_line":374,"start_character":0,"end_line":374,"end_character":51},"in_reply_to":"ff570b3c_1b247fae","updated":"2020-05-20 09:45:48.000000000","message":"Nope. \nThe point is that here the file is already written by rbd export (hence \u0027rb\u0027), while the other part opens it for writing.\nWe are only opening the written file again because of the verification bits","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                        # for verification (if required)"},{"line_number":374,"context_line":"                        data \u003d open(dst_path, \u0027rb\u0027)"},{"line_number":375,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":376,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":377,"context_line":"                            downloaded_length)"},{"line_number":378,"context_line":"                    except Exception:"},{"line_number":379,"context_line":"                        LOG.exception(\"Download image error\")"},{"line_number":380,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_183da5dc","line":377,"range":{"start_line":376,"start_character":0,"end_line":377,"end_character":46},"updated":"2020-05-19 15:29:10.000000000","message":"Could we use this on L401?\n\n  if image_chunks is None:\n      image_chunks \u003d glance_utils.IterableWithLength(\n          data, os.path.getsize(dst_path))","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5083c5a50c2111707b11940e660ef5ce92252bfe","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                        # for verification (if required)"},{"line_number":374,"context_line":"                        data \u003d open(dst_path, \u0027rb\u0027)"},{"line_number":375,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":376,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":377,"context_line":"                            downloaded_length)"},{"line_number":378,"context_line":"                    except Exception:"},{"line_number":379,"context_line":"                        LOG.exception(\"Download image error\")"},{"line_number":380,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_d55da079","line":377,"range":{"start_line":376,"start_character":0,"end_line":377,"end_character":46},"in_reply_to":"ff570b3c_183da5dc","updated":"2020-05-20 09:45:48.000000000","message":"Why? \nimage_chunks is intentionally initialized here so it does not get filled in L381-392 (via the glance API call)","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"4d4724f7c5288bc3732e242d0b41f81f0fcae90c","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                        # for verification (if required)"},{"line_number":374,"context_line":"                        data \u003d open(dst_path, \u0027rb\u0027)"},{"line_number":375,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":376,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":377,"context_line":"                            downloaded_length)"},{"line_number":378,"context_line":"                    except Exception:"},{"line_number":379,"context_line":"                        LOG.exception(\"Download image error\")"},{"line_number":380,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_27e66fe9","line":377,"range":{"start_line":376,"start_character":0,"end_line":377,"end_character":46},"in_reply_to":"ff570b3c_d55da079","updated":"2020-05-20 10:23:40.000000000","message":"Yeah apologies I somehow missed that.","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":399,"context_line":"            data \u003d open(dst_path, \u0027wb\u0027)"},{"line_number":400,"context_line":"            close_file \u003d True"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        if data is None or downloaded_length:"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"            # Perform image signature verification"},{"line_number":405,"context_line":"            if verifier:"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_b82d99a9","line":402,"range":{"start_line":402,"start_character":24,"end_line":402,"end_character":44},"updated":"2020-05-19 15:29:10.000000000","message":"and image_chunks","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5083c5a50c2111707b11940e660ef5ce92252bfe","unresolved":false,"context_lines":[{"line_number":399,"context_line":"            data \u003d open(dst_path, \u0027wb\u0027)"},{"line_number":400,"context_line":"            close_file \u003d True"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        if data is None or downloaded_length:"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"            # Perform image signature verification"},{"line_number":405,"context_line":"            if verifier:"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_b5c90c27","line":402,"range":{"start_line":402,"start_character":24,"end_line":402,"end_character":44},"in_reply_to":"ff570b3c_b82d99a9","updated":"2020-05-20 09:45:48.000000000","message":"Why?","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":415,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":416,"context_line":"                        LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":417,"context_line":"                                  \u0027for image: %s\u0027, image_id)"},{"line_number":418,"context_line":"            if data is None:"},{"line_number":419,"context_line":"                return image_chunks"},{"line_number":420,"context_line":"        else:"},{"line_number":421,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_38134968","line":418,"range":{"start_line":418,"start_character":0,"end_line":418,"end_character":28},"updated":"2020-05-19 15:29:10.000000000","message":"Seems redundant given L402","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5083c5a50c2111707b11940e660ef5ce92252bfe","unresolved":false,"context_lines":[{"line_number":415,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":416,"context_line":"                        LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":417,"context_line":"                                  \u0027for image: %s\u0027, image_id)"},{"line_number":418,"context_line":"            if data is None:"},{"line_number":419,"context_line":"                return image_chunks"},{"line_number":420,"context_line":"        else:"},{"line_number":421,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_959a0811","line":418,"range":{"start_line":418,"start_character":0,"end_line":418,"end_character":28},"in_reply_to":"ff570b3c_38134968","updated":"2020-05-20 09:45:48.000000000","message":"It\u0027s bit weird, but the point is (I think) to leave this behavior (i.e. returning image_chunks from this method) to the same case where it was before patching. (I honestly don\u0027t know why was it there)\n\nBut you can see that this patch extends this \"# Perform image signature verification\" code with one special case, the one where we downloaded the image using extra handler. AFAICS the point was to reuse the verification code, not the return part","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"83efa50be705fe9e4ef9c703d792ca3d3743bb23","unresolved":false,"context_lines":[{"line_number":259,"context_line":"                      \"[snapshot_name: %s] [dst_path: %s]\","},{"line_number":260,"context_line":"                      pool_name, image_uuid, snapshot_name, dst_path)"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"            rbd_driver.export_image("},{"line_number":263,"context_line":"                dst_path,"},{"line_number":264,"context_line":"                image_uuid,"},{"line_number":265,"context_line":"                snapshot_name,"},{"line_number":266,"context_line":"                pool_name)"},{"line_number":267,"context_line":"        except Exception as e:"},{"line_number":268,"context_line":"            LOG.error(\"Error during RBD image export: %s\", e)"},{"line_number":269,"context_line":"            raise nova.exception.CouldNotFetchImage(image_id\u003dimage_uuid)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ff570b3c_fd01aa6b","line":266,"range":{"start_line":262,"start_character":0,"end_line":266,"end_character":26},"updated":"2020-05-20 11:19:30.000000000","message":"supernit - We don\u0027t need to take up this much vertical space here IMHO.","commit_id":"ea4f22bb604534665dc4c3b58ecc6e1cd158d3a4"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"83efa50be705fe9e4ef9c703d792ca3d3743bb23","unresolved":false,"context_lines":[{"line_number":361,"context_line":"                        xfer_method(context, o, dst_path, loc_meta)"},{"line_number":362,"context_line":"                        LOG.info(\"Successfully transferred using %s\", o.scheme)"},{"line_number":363,"context_line":""},{"line_number":364,"context_line":"                        # Load chunks from the downloaded image file"},{"line_number":365,"context_line":"                        # for verification (if required)"},{"line_number":366,"context_line":"                        data \u003d open(dst_path, \u0027rb\u0027)"},{"line_number":367,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":368,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":369,"context_line":"                            downloaded_length)"},{"line_number":370,"context_line":"                    except Exception:"},{"line_number":371,"context_line":"                        LOG.exception(\"Download image error\")"},{"line_number":372,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"ff570b3c_fdb36ae7","line":369,"range":{"start_line":364,"start_character":0,"end_line":369,"end_character":46},"updated":"2020-05-20 11:19:30.000000000","message":"Given you\u0027re the first to implement a handler here would it make sense to include this in the method itself and have it return data and image_chunks?","commit_id":"ea4f22bb604534665dc4c3b58ecc6e1cd158d3a4"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"586df18900fcd0ff440e72d2d7be02ff3cb99c3d","unresolved":false,"context_lines":[{"line_number":361,"context_line":"                        xfer_method(context, o, dst_path, loc_meta)"},{"line_number":362,"context_line":"                        LOG.info(\"Successfully transferred using %s\", o.scheme)"},{"line_number":363,"context_line":""},{"line_number":364,"context_line":"                        # Load chunks from the downloaded image file"},{"line_number":365,"context_line":"                        # for verification (if required)"},{"line_number":366,"context_line":"                        data \u003d open(dst_path, \u0027rb\u0027)"},{"line_number":367,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":368,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":369,"context_line":"                            downloaded_length)"},{"line_number":370,"context_line":"                    except Exception:"},{"line_number":371,"context_line":"                        LOG.exception(\"Download image error\")"},{"line_number":372,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"ff570b3c_e0ebbd0f","line":369,"range":{"start_line":364,"start_character":0,"end_line":369,"end_character":46},"in_reply_to":"ff570b3c_fdb36ae7","updated":"2020-05-20 11:57:07.000000000","message":"Hm, maybe.\nIs it a good idea to just let the file object be passed around?","commit_id":"ea4f22bb604534665dc4c3b58ecc6e1cd158d3a4"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"83efa50be705fe9e4ef9c703d792ca3d3743bb23","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                raise exception.ImageUnacceptable(image_id\u003dimage_id,"},{"line_number":384,"context_line":"                    reason\u003d\u0027Image has no associated data\u0027)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"        # Retrieve properties for verification of Glance image signature"},{"line_number":387,"context_line":"        verifier \u003d self._get_verifier(context, image_id, trusted_certs)"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        close_file \u003d False"},{"line_number":390,"context_line":"        if data is None and dst_path:"},{"line_number":391,"context_line":"            data \u003d open(dst_path, \u0027wb\u0027)"},{"line_number":392,"context_line":"            close_file \u003d True"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        if data is None or downloaded_length:"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"            # Perform image signature verification"},{"line_number":397,"context_line":"            if verifier:"},{"line_number":398,"context_line":"                try:"},{"line_number":399,"context_line":"                    for chunk in image_chunks:"},{"line_number":400,"context_line":"                        verifier.update(chunk)"},{"line_number":401,"context_line":"                    verifier.verify()"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"                    LOG.info(\u0027Image signature verification succeeded \u0027"},{"line_number":404,"context_line":"                             \u0027for image: %s\u0027, image_id)"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"                except cryptography.exceptions.InvalidSignature:"},{"line_number":407,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":408,"context_line":"                        LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":409,"context_line":"                                  \u0027for image: %s\u0027, image_id)"},{"line_number":410,"context_line":"            if data is None:"},{"line_number":411,"context_line":"                return image_chunks"},{"line_number":412,"context_line":"        else:"},{"line_number":413,"context_line":"            try:"},{"line_number":414,"context_line":"                for chunk in image_chunks:"},{"line_number":415,"context_line":"                    if verifier:"},{"line_number":416,"context_line":"                        verifier.update(chunk)"},{"line_number":417,"context_line":"                    data.write(chunk)"},{"line_number":418,"context_line":"                if verifier:"},{"line_number":419,"context_line":"                    verifier.verify()"},{"line_number":420,"context_line":"                    LOG.info(\u0027Image signature verification succeeded \u0027"},{"line_number":421,"context_line":"                             \u0027for image %s\u0027, image_id)"},{"line_number":422,"context_line":"            except cryptography.exceptions.InvalidSignature:"},{"line_number":423,"context_line":"                data.truncate(0)"},{"line_number":424,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":425,"context_line":"                    LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":426,"context_line":"                              \u0027for image: %s\u0027, image_id)"},{"line_number":427,"context_line":"            except Exception as ex:"},{"line_number":428,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":429,"context_line":"                    LOG.error(\"Error writing to %(path)s: %(exception)s\","},{"line_number":430,"context_line":"                              {\u0027path\u0027: dst_path, \u0027exception\u0027: ex})"},{"line_number":431,"context_line":"            finally:"},{"line_number":432,"context_line":"                if close_file:"},{"line_number":433,"context_line":"                    # Ensure that the data is pushed all the way down to"},{"line_number":434,"context_line":"                    # persistent storage. This ensures that in the event of a"},{"line_number":435,"context_line":"                    # subsequent host crash we don\u0027t have running instances"},{"line_number":436,"context_line":"                    # using a corrupt backing file."},{"line_number":437,"context_line":"                    data.flush()"},{"line_number":438,"context_line":"                    self._safe_fsync(data)"},{"line_number":439,"context_line":"                    data.close()"},{"line_number":440,"context_line":""},{"line_number":441,"context_line":"    def _get_verifier(self, context, image_id, trusted_certs):"},{"line_number":442,"context_line":"        verifier \u003d None"}],"source_content_type":"text/x-python","patch_set":20,"id":"ff570b3c_ca685af6","line":439,"range":{"start_line":386,"start_character":0,"end_line":439,"end_character":32},"updated":"2020-05-20 11:19:30.000000000","message":"I\u0027d prefer that we flatten this into a single private _verify_download method that handles both verification cases, one where we need to write out the file during verification and where we don\u0027t.","commit_id":"ea4f22bb604534665dc4c3b58ecc6e1cd158d3a4"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"586df18900fcd0ff440e72d2d7be02ff3cb99c3d","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                raise exception.ImageUnacceptable(image_id\u003dimage_id,"},{"line_number":384,"context_line":"                    reason\u003d\u0027Image has no associated data\u0027)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"        # Retrieve properties for verification of Glance image signature"},{"line_number":387,"context_line":"        verifier \u003d self._get_verifier(context, image_id, trusted_certs)"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        close_file \u003d False"},{"line_number":390,"context_line":"        if data is None and dst_path:"},{"line_number":391,"context_line":"            data \u003d open(dst_path, \u0027wb\u0027)"},{"line_number":392,"context_line":"            close_file \u003d True"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        if data is None or downloaded_length:"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"            # Perform image signature verification"},{"line_number":397,"context_line":"            if verifier:"},{"line_number":398,"context_line":"                try:"},{"line_number":399,"context_line":"                    for chunk in image_chunks:"},{"line_number":400,"context_line":"                        verifier.update(chunk)"},{"line_number":401,"context_line":"                    verifier.verify()"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"                    LOG.info(\u0027Image signature verification succeeded \u0027"},{"line_number":404,"context_line":"                             \u0027for image: %s\u0027, image_id)"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"                except cryptography.exceptions.InvalidSignature:"},{"line_number":407,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":408,"context_line":"                        LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":409,"context_line":"                                  \u0027for image: %s\u0027, image_id)"},{"line_number":410,"context_line":"            if data is None:"},{"line_number":411,"context_line":"                return image_chunks"},{"line_number":412,"context_line":"        else:"},{"line_number":413,"context_line":"            try:"},{"line_number":414,"context_line":"                for chunk in image_chunks:"},{"line_number":415,"context_line":"                    if verifier:"},{"line_number":416,"context_line":"                        verifier.update(chunk)"},{"line_number":417,"context_line":"                    data.write(chunk)"},{"line_number":418,"context_line":"                if verifier:"},{"line_number":419,"context_line":"                    verifier.verify()"},{"line_number":420,"context_line":"                    LOG.info(\u0027Image signature verification succeeded \u0027"},{"line_number":421,"context_line":"                             \u0027for image %s\u0027, image_id)"},{"line_number":422,"context_line":"            except cryptography.exceptions.InvalidSignature:"},{"line_number":423,"context_line":"                data.truncate(0)"},{"line_number":424,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":425,"context_line":"                    LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":426,"context_line":"                              \u0027for image: %s\u0027, image_id)"},{"line_number":427,"context_line":"            except Exception as ex:"},{"line_number":428,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":429,"context_line":"                    LOG.error(\"Error writing to %(path)s: %(exception)s\","},{"line_number":430,"context_line":"                              {\u0027path\u0027: dst_path, \u0027exception\u0027: ex})"},{"line_number":431,"context_line":"            finally:"},{"line_number":432,"context_line":"                if close_file:"},{"line_number":433,"context_line":"                    # Ensure that the data is pushed all the way down to"},{"line_number":434,"context_line":"                    # persistent storage. This ensures that in the event of a"},{"line_number":435,"context_line":"                    # subsequent host crash we don\u0027t have running instances"},{"line_number":436,"context_line":"                    # using a corrupt backing file."},{"line_number":437,"context_line":"                    data.flush()"},{"line_number":438,"context_line":"                    self._safe_fsync(data)"},{"line_number":439,"context_line":"                    data.close()"},{"line_number":440,"context_line":""},{"line_number":441,"context_line":"    def _get_verifier(self, context, image_id, trusted_certs):"},{"line_number":442,"context_line":"        verifier \u003d None"}],"source_content_type":"text/x-python","patch_set":20,"id":"ff570b3c_40ac29ec","line":439,"range":{"start_line":386,"start_character":0,"end_line":439,"end_character":32},"in_reply_to":"ff570b3c_ca685af6","updated":"2020-05-20 11:57:07.000000000","message":"OK, I\u0027ll take a look at it ...","commit_id":"ea4f22bb604534665dc4c3b58ecc6e1cd158d3a4"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d8d79d753080f7bd7f8739c7af3230feff0960e6","unresolved":false,"context_lines":[{"line_number":357,"context_line":"                        xfer_method(context, o, dst_path, loc_meta)"},{"line_number":358,"context_line":"                        LOG.info(\"Successfully transferred using %s\", o.scheme)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"                        # Load chunks from the downloaded image file"},{"line_number":361,"context_line":"                        # for verification (if required)"},{"line_number":362,"context_line":"                        data \u003d open(dst_path, \u0027rb\u0027)"},{"line_number":363,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":364,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":365,"context_line":"                            downloaded_length)"},{"line_number":366,"context_line":"                        return self._verify_and_write(context,"},{"line_number":367,"context_line":"                                                      image_id,"},{"line_number":368,"context_line":"                                                      trusted_certs,"}],"source_content_type":"text/x-python","patch_set":22,"id":"ff570b3c_359dc766","line":365,"range":{"start_line":360,"start_character":0,"end_line":365,"end_character":46},"updated":"2020-05-22 14:04:06.000000000","message":"I still think this is something we could force xfer_method methods to return above but that could just be something we refactor if any new methods are added in the future so feel free to ignore for now.","commit_id":"318e5d7b12ec711a7b7bc4898ebeb8a32f58c2ad"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5a96954bac6c460ff11a74530ab2621d42de76d3","unresolved":false,"context_lines":[{"line_number":357,"context_line":"                        xfer_method(context, o, dst_path, loc_meta)"},{"line_number":358,"context_line":"                        LOG.info(\"Successfully transferred using %s\", o.scheme)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"                        # Load chunks from the downloaded image file"},{"line_number":361,"context_line":"                        # for verification (if required)"},{"line_number":362,"context_line":"                        data \u003d open(dst_path, \u0027rb\u0027)"},{"line_number":363,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":364,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":365,"context_line":"                            downloaded_length)"},{"line_number":366,"context_line":"                        return self._verify_and_write(context,"},{"line_number":367,"context_line":"                                                      image_id,"},{"line_number":368,"context_line":"                                                      trusted_certs,"}],"source_content_type":"text/x-python","patch_set":22,"id":"ff570b3c_30aaf518","line":365,"range":{"start_line":360,"start_character":0,"end_line":365,"end_character":46},"in_reply_to":"ff570b3c_359dc766","updated":"2020-05-22 14:32:49.000000000","message":"Yeah, I don\u0027t think it would really add much value","commit_id":"318e5d7b12ec711a7b7bc4898ebeb8a32f58c2ad"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d8d79d753080f7bd7f8739c7af3230feff0960e6","unresolved":false,"context_lines":[{"line_number":363,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":364,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":365,"context_line":"                            downloaded_length)"},{"line_number":366,"context_line":"                        return self._verify_and_write(context,"},{"line_number":367,"context_line":"                                                      image_id,"},{"line_number":368,"context_line":"                                                      trusted_certs,"},{"line_number":369,"context_line":"                                                      image_chunks,"},{"line_number":370,"context_line":"                                                      data,"},{"line_number":371,"context_line":"                                                      dst_path,"},{"line_number":372,"context_line":"                                                      False)"},{"line_number":373,"context_line":"                    except Exception:"},{"line_number":374,"context_line":"                        LOG.exception(\"Download image error\")"},{"line_number":375,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"ff570b3c_55095b93","line":372,"range":{"start_line":366,"start_character":0,"end_line":372,"end_character":60},"updated":"2020-05-22 14:04:06.000000000","message":"Can you collapse this down into fewer lines?\n\n                        return self._verify_and_write(\n                            context, image_id, trusted_certs, image_chunks,\n                            data, dst_path, False)","commit_id":"318e5d7b12ec711a7b7bc4898ebeb8a32f58c2ad"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5a96954bac6c460ff11a74530ab2621d42de76d3","unresolved":false,"context_lines":[{"line_number":363,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":364,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":365,"context_line":"                            downloaded_length)"},{"line_number":366,"context_line":"                        return self._verify_and_write(context,"},{"line_number":367,"context_line":"                                                      image_id,"},{"line_number":368,"context_line":"                                                      trusted_certs,"},{"line_number":369,"context_line":"                                                      image_chunks,"},{"line_number":370,"context_line":"                                                      data,"},{"line_number":371,"context_line":"                                                      dst_path,"},{"line_number":372,"context_line":"                                                      False)"},{"line_number":373,"context_line":"                    except Exception:"},{"line_number":374,"context_line":"                        LOG.exception(\"Download image error\")"},{"line_number":375,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"ff570b3c_b0b6058a","line":372,"range":{"start_line":366,"start_character":0,"end_line":372,"end_character":60},"in_reply_to":"ff570b3c_55095b93","updated":"2020-05-22 14:32:49.000000000","message":"Sure","commit_id":"318e5d7b12ec711a7b7bc4898ebeb8a32f58c2ad"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d8d79d753080f7bd7f8739c7af3230feff0960e6","unresolved":false,"context_lines":[{"line_number":391,"context_line":"                                      image_chunks, data, dst_path, True)"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"    def _verify_and_write(self, context, image_id, trusted_certs,"},{"line_number":394,"context_line":"                          image_chunks, data, dst_path, write_it):"},{"line_number":395,"context_line":"        \"\"\"Perform image signature verification and save the image file if needed."},{"line_number":396,"context_line":"        \"\"\""},{"line_number":397,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"ff570b3c_75dbbf20","line":394,"range":{"start_line":394,"start_character":56,"end_line":394,"end_character":64},"updated":"2020-05-22 14:04:06.000000000","message":"nit - write_image would make more sense, docstrings would also be ace if you have the time.","commit_id":"318e5d7b12ec711a7b7bc4898ebeb8a32f58c2ad"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5a96954bac6c460ff11a74530ab2621d42de76d3","unresolved":false,"context_lines":[{"line_number":391,"context_line":"                                      image_chunks, data, dst_path, True)"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"    def _verify_and_write(self, context, image_id, trusted_certs,"},{"line_number":394,"context_line":"                          image_chunks, data, dst_path, write_it):"},{"line_number":395,"context_line":"        \"\"\"Perform image signature verification and save the image file if needed."},{"line_number":396,"context_line":"        \"\"\""},{"line_number":397,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"ff570b3c_10037111","line":394,"range":{"start_line":394,"start_character":56,"end_line":394,"end_character":64},"in_reply_to":"ff570b3c_75dbbf20","updated":"2020-05-22 14:32:49.000000000","message":"OK","commit_id":"318e5d7b12ec711a7b7bc4898ebeb8a32f58c2ad"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d8d79d753080f7bd7f8739c7af3230feff0960e6","unresolved":false,"context_lines":[{"line_number":405,"context_line":""},{"line_number":406,"context_line":"        # Retrieve properties for verification of Glance image signature"},{"line_number":407,"context_line":"        verifier \u003d self._get_verifier(context, image_id, trusted_certs)"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"        try:"},{"line_number":410,"context_line":"            for chunk in image_chunks:"},{"line_number":411,"context_line":"                if verifier:"}],"source_content_type":"text/x-python","patch_set":22,"id":"ff570b3c_90b941eb","line":408,"updated":"2020-05-22 14:04:06.000000000","message":"Would it make sense to bail out here if we don\u0027t need to write or verify the image?\n\n  # We don\u0027t need to write or verify the image\n  if write_it is False and verifier is None:\n      if data:\n          return \n      else:\n          return image_chunks","commit_id":"318e5d7b12ec711a7b7bc4898ebeb8a32f58c2ad"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5a96954bac6c460ff11a74530ab2621d42de76d3","unresolved":false,"context_lines":[{"line_number":405,"context_line":""},{"line_number":406,"context_line":"        # Retrieve properties for verification of Glance image signature"},{"line_number":407,"context_line":"        verifier \u003d self._get_verifier(context, image_id, trusted_certs)"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"        try:"},{"line_number":410,"context_line":"            for chunk in image_chunks:"},{"line_number":411,"context_line":"                if verifier:"}],"source_content_type":"text/x-python","patch_set":22,"id":"ff570b3c_2b6286d9","line":408,"in_reply_to":"ff570b3c_90b941eb","updated":"2020-05-22 14:32:49.000000000","message":"Yeah, good point, I actually had it in some previous version than I\u0027ve removed it as it seemed like it is changing some test results","commit_id":"318e5d7b12ec711a7b7bc4898ebeb8a32f58c2ad"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d8d79d753080f7bd7f8739c7af3230feff0960e6","unresolved":false,"context_lines":[{"line_number":423,"context_line":"                LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":424,"context_line":"                          \u0027for image %s\u0027, image_id)"},{"line_number":425,"context_line":"        except Exception as ex:"},{"line_number":426,"context_line":"            if write_it:"},{"line_number":427,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":428,"context_line":"                    LOG.error(\"Error writing to %(path)s: %(exception)s\","},{"line_number":429,"context_line":"                              {\u0027path\u0027: dst_path, \u0027exception\u0027: ex})"}],"source_content_type":"text/x-python","patch_set":22,"id":"ff570b3c_d59d6b58","line":426,"range":{"start_line":426,"start_character":0,"end_line":426,"end_character":24},"updated":"2020-05-22 14:04:06.000000000","message":"Only logging this when we are writing didn\u0027t make sense to me in the original tbh, I\u0027d rather drop this and make the LOG.error below more generic.","commit_id":"318e5d7b12ec711a7b7bc4898ebeb8a32f58c2ad"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5a96954bac6c460ff11a74530ab2621d42de76d3","unresolved":false,"context_lines":[{"line_number":423,"context_line":"                LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":424,"context_line":"                          \u0027for image %s\u0027, image_id)"},{"line_number":425,"context_line":"        except Exception as ex:"},{"line_number":426,"context_line":"            if write_it:"},{"line_number":427,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":428,"context_line":"                    LOG.error(\"Error writing to %(path)s: %(exception)s\","},{"line_number":429,"context_line":"                              {\u0027path\u0027: dst_path, \u0027exception\u0027: ex})"}],"source_content_type":"text/x-python","patch_set":22,"id":"ff570b3c_eb67eee9","line":426,"range":{"start_line":426,"start_character":0,"end_line":426,"end_character":24},"in_reply_to":"ff570b3c_d59d6b58","updated":"2020-05-22 14:32:49.000000000","message":"Well ... I think the original code somewhat implied that this really should be only writing specific error, as we tried to catch verification specific exceptions before ... I know, there\u0027s just Exception so theoretically it could be more.\n\nBut more generic? \"Error verifying or writing\" ? Sounds weird...","commit_id":"318e5d7b12ec711a7b7bc4898ebeb8a32f58c2ad"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2cf754cdb2aca0fd29f07303b203502b7ab85a03","unresolved":false,"context_lines":[{"line_number":237,"context_line":"        # NOTE(jsuchome): It is pretty ugly to import the module here,"},{"line_number":238,"context_line":"        # however if we\u0027d have it on the usual place, it would bring circular"},{"line_number":239,"context_line":"        # dependency (libvirt.storage imports image.glance via virt.images)"},{"line_number":240,"context_line":"        # which is not easy to break without larger refactoring."},{"line_number":241,"context_line":"        from nova.virt.libvirt.storage import rbd_utils"},{"line_number":242,"context_line":"        try:"},{"line_number":243,"context_line":"            # Parse the RBD URL from url_parts, it should consist of 4"}],"source_content_type":"text/x-python","patch_set":24,"id":"ff570b3c_b7949b45","line":240,"updated":"2020-06-10 15:01:45.000000000","message":"You can just say \"Avoid circular import\" and everyone will know what you mean. In a thing as complex as nova, it\u0027s sometimes unavoidable and we have many such cases elsewhere in the code.","commit_id":"3401a2fe402c2e205623221fc6a720f02596237f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d6f2770f38b5bbe00c031a93c72fe768aa5806e3","unresolved":false,"context_lines":[{"line_number":237,"context_line":"        # NOTE(jsuchome): It is pretty ugly to import the module here,"},{"line_number":238,"context_line":"        # however if we\u0027d have it on the usual place, it would bring circular"},{"line_number":239,"context_line":"        # dependency (libvirt.storage imports image.glance via virt.images)"},{"line_number":240,"context_line":"        # which is not easy to break without larger refactoring."},{"line_number":241,"context_line":"        from nova.virt.libvirt.storage import rbd_utils"},{"line_number":242,"context_line":"        try:"},{"line_number":243,"context_line":"            # Parse the RBD URL from url_parts, it should consist of 4"}],"source_content_type":"text/x-python","patch_set":24,"id":"ff570b3c_656118a9","line":240,"in_reply_to":"ff570b3c_05dbfc7c","updated":"2020-06-10 21:06:00.000000000","message":"Also, he just asked for the actual reason for the circular import, which (as noted) I don\u0027t think is particularly common for us to do, but even still, people know what a circular import is, so you can just note the other thing here. However, in this case, I expect a lot of places import this since it\u0027s base functionality.","commit_id":"3401a2fe402c2e205623221fc6a720f02596237f"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"179c0ab5d5ecf97b805d35092fda3239deb1b71c","unresolved":false,"context_lines":[{"line_number":237,"context_line":"        # NOTE(jsuchome): It is pretty ugly to import the module here,"},{"line_number":238,"context_line":"        # however if we\u0027d have it on the usual place, it would bring circular"},{"line_number":239,"context_line":"        # dependency (libvirt.storage imports image.glance via virt.images)"},{"line_number":240,"context_line":"        # which is not easy to break without larger refactoring."},{"line_number":241,"context_line":"        from nova.virt.libvirt.storage import rbd_utils"},{"line_number":242,"context_line":"        try:"},{"line_number":243,"context_line":"            # Parse the RBD URL from url_parts, it should consist of 4"}],"source_content_type":"text/x-python","patch_set":24,"id":"ff570b3c_a41699f0","line":240,"in_reply_to":"ff570b3c_447cdd71","updated":"2020-06-10 16:04:37.000000000","message":"It was Lee, reviewing Patch Set 18","commit_id":"3401a2fe402c2e205623221fc6a720f02596237f"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"d86d748af9d70836dd7f33c19d208af60dc6dda4","unresolved":false,"context_lines":[{"line_number":237,"context_line":"        # NOTE(jsuchome): It is pretty ugly to import the module here,"},{"line_number":238,"context_line":"        # however if we\u0027d have it on the usual place, it would bring circular"},{"line_number":239,"context_line":"        # dependency (libvirt.storage imports image.glance via virt.images)"},{"line_number":240,"context_line":"        # which is not easy to break without larger refactoring."},{"line_number":241,"context_line":"        from nova.virt.libvirt.storage import rbd_utils"},{"line_number":242,"context_line":"        try:"},{"line_number":243,"context_line":"            # Parse the RBD URL from url_parts, it should consist of 4"}],"source_content_type":"text/x-python","patch_set":24,"id":"ff570b3c_e8c4df43","line":240,"in_reply_to":"ff570b3c_656118a9","updated":"2020-06-10 21:50:12.000000000","message":"I\u0027m not sure I understand - is it wanted then or not?\nHow do you like it to look? Just drop the specific example libvirt.storage ?\nI could just revert it to the original version of comment...","commit_id":"3401a2fe402c2e205623221fc6a720f02596237f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3a92142d82bf79be9e24e61452fc6ff8e65820af","unresolved":false,"context_lines":[{"line_number":237,"context_line":"        # NOTE(jsuchome): It is pretty ugly to import the module here,"},{"line_number":238,"context_line":"        # however if we\u0027d have it on the usual place, it would bring circular"},{"line_number":239,"context_line":"        # dependency (libvirt.storage imports image.glance via virt.images)"},{"line_number":240,"context_line":"        # which is not easy to break without larger refactoring."},{"line_number":241,"context_line":"        from nova.virt.libvirt.storage import rbd_utils"},{"line_number":242,"context_line":"        try:"},{"line_number":243,"context_line":"            # Parse the RBD URL from url_parts, it should consist of 4"}],"source_content_type":"text/x-python","patch_set":24,"id":"ff570b3c_05dbfc7c","line":240,"in_reply_to":"ff570b3c_a41699f0","updated":"2020-06-10 21:03:53.000000000","message":"Just the first few examples I found:\n\nhttps://github.com/openstack/nova/blob/master/nova/virt/driver.py#L968\n\nhttps://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L3258\n\nhttps://github.com/openstack/nova/blob/master/nova/context.py#L342","commit_id":"3401a2fe402c2e205623221fc6a720f02596237f"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"ec4515c3637d128ed81dfa0b722ae5f1c53ed906","unresolved":false,"context_lines":[{"line_number":237,"context_line":"        # NOTE(jsuchome): It is pretty ugly to import the module here,"},{"line_number":238,"context_line":"        # however if we\u0027d have it on the usual place, it would bring circular"},{"line_number":239,"context_line":"        # dependency (libvirt.storage imports image.glance via virt.images)"},{"line_number":240,"context_line":"        # which is not easy to break without larger refactoring."},{"line_number":241,"context_line":"        from nova.virt.libvirt.storage import rbd_utils"},{"line_number":242,"context_line":"        try:"},{"line_number":243,"context_line":"            # Parse the RBD URL from url_parts, it should consist of 4"}],"source_content_type":"text/x-python","patch_set":24,"id":"ff570b3c_447cdd71","line":240,"in_reply_to":"ff570b3c_b7949b45","updated":"2020-06-10 16:03:38.000000000","message":"Well, in someone in previous review asked that I elaborate here...","commit_id":"3401a2fe402c2e205623221fc6a720f02596237f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2cf754cdb2aca0fd29f07303b203502b7ab85a03","unresolved":false,"context_lines":[{"line_number":378,"context_line":""},{"line_number":379,"context_line":"        if image_chunks.wrapped is None:"},{"line_number":380,"context_line":"            # None is a valid return value, but there\u0027s nothing we can do"},{"line_number":381,"context_line":"            # with an image with no associated data"},{"line_number":382,"context_line":"            raise exception.ImageUnacceptable(image_id\u003dimage_id,"},{"line_number":383,"context_line":"                reason\u003d\u0027Image has no associated data\u0027)"},{"line_number":384,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"ff570b3c_b7cbfb50","line":381,"updated":"2020-06-10 15:01:45.000000000","message":"This is a non-functional change which just introduces git noise. Please put this back the way it was to avoid it looking like this patch introduced this comment.","commit_id":"3401a2fe402c2e205623221fc6a720f02596237f"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"ec4515c3637d128ed81dfa0b722ae5f1c53ed906","unresolved":false,"context_lines":[{"line_number":378,"context_line":""},{"line_number":379,"context_line":"        if image_chunks.wrapped is None:"},{"line_number":380,"context_line":"            # None is a valid return value, but there\u0027s nothing we can do"},{"line_number":381,"context_line":"            # with an image with no associated data"},{"line_number":382,"context_line":"            raise exception.ImageUnacceptable(image_id\u003dimage_id,"},{"line_number":383,"context_line":"                reason\u003d\u0027Image has no associated data\u0027)"},{"line_number":384,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"ff570b3c_04d02553","line":381,"in_reply_to":"ff570b3c_b7cbfb50","updated":"2020-06-10 16:03:38.000000000","message":"Hm... OK. I thought that I had to do this because pep8 complained or something. Let\u0027s see...","commit_id":"3401a2fe402c2e205623221fc6a720f02596237f"},{"author":{"_account_id":11347,"name":"Rui Zang","email":"rui.zang@yandex.com","username":"rzang"},"change_message_id":"9de8d13a3db52991bca84bb35802d11b9efff337","unresolved":false,"context_lines":[{"line_number":418,"context_line":"                return"},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"        try:"},{"line_number":421,"context_line":"            for chunk in image_chunks:"},{"line_number":422,"context_line":"                if verifier:"},{"line_number":423,"context_line":"                    verifier.update(chunk)"},{"line_number":424,"context_line":"                if write_image:"},{"line_number":425,"context_line":"                    data.write(chunk)"},{"line_number":426,"context_line":"            if verifier:"},{"line_number":427,"context_line":"                verifier.verify()"},{"line_number":428,"context_line":"                LOG.info(\u0027Image signature verification succeeded \u0027"},{"line_number":429,"context_line":"                         \u0027for image %s\u0027, image_id)"},{"line_number":430,"context_line":"        except cryptography.exceptions.InvalidSignature:"},{"line_number":431,"context_line":"            if write_image:"},{"line_number":432,"context_line":"                data.truncate(0)"}],"source_content_type":"text/x-python","patch_set":24,"id":"ff570b3c_63616cd3","line":429,"range":{"start_line":421,"start_character":0,"end_line":429,"end_character":50},"updated":"2020-06-05 02:59:08.000000000","message":"How about first do the verification and then another round of iteration of image_chunks for writing? This will save unnecessary disk IOs in case the verification fails.","commit_id":"3401a2fe402c2e205623221fc6a720f02596237f"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"ec4515c3637d128ed81dfa0b722ae5f1c53ed906","unresolved":false,"context_lines":[{"line_number":418,"context_line":"                return"},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"        try:"},{"line_number":421,"context_line":"            for chunk in image_chunks:"},{"line_number":422,"context_line":"                if verifier:"},{"line_number":423,"context_line":"                    verifier.update(chunk)"},{"line_number":424,"context_line":"                if write_image:"},{"line_number":425,"context_line":"                    data.write(chunk)"},{"line_number":426,"context_line":"            if verifier:"},{"line_number":427,"context_line":"                verifier.verify()"},{"line_number":428,"context_line":"                LOG.info(\u0027Image signature verification succeeded \u0027"},{"line_number":429,"context_line":"                         \u0027for image %s\u0027, image_id)"},{"line_number":430,"context_line":"        except cryptography.exceptions.InvalidSignature:"},{"line_number":431,"context_line":"            if write_image:"},{"line_number":432,"context_line":"                data.truncate(0)"}],"source_content_type":"text/x-python","patch_set":24,"id":"ff570b3c_646e4197","line":429,"range":{"start_line":421,"start_character":0,"end_line":429,"end_character":50},"in_reply_to":"ff570b3c_63616cd3","updated":"2020-06-10 16:03:38.000000000","message":"I can do that, though do you really think it would be better? It could slow us little bit","commit_id":"3401a2fe402c2e205623221fc6a720f02596237f"},{"author":{"_account_id":11347,"name":"Rui Zang","email":"rui.zang@yandex.com","username":"rzang"},"change_message_id":"72a37e2c8df6fb0ae7aef90056a754bdc2ace0a2","unresolved":false,"context_lines":[{"line_number":418,"context_line":"                return"},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"        try:"},{"line_number":421,"context_line":"            for chunk in image_chunks:"},{"line_number":422,"context_line":"                if verifier:"},{"line_number":423,"context_line":"                    verifier.update(chunk)"},{"line_number":424,"context_line":"                if write_image:"},{"line_number":425,"context_line":"                    data.write(chunk)"},{"line_number":426,"context_line":"            if verifier:"},{"line_number":427,"context_line":"                verifier.verify()"},{"line_number":428,"context_line":"                LOG.info(\u0027Image signature verification succeeded \u0027"},{"line_number":429,"context_line":"                         \u0027for image %s\u0027, image_id)"},{"line_number":430,"context_line":"        except cryptography.exceptions.InvalidSignature:"},{"line_number":431,"context_line":"            if write_image:"},{"line_number":432,"context_line":"                data.truncate(0)"}],"source_content_type":"text/x-python","patch_set":24,"id":"bf51134e_4aed2aee","line":429,"range":{"start_line":421,"start_character":0,"end_line":429,"end_character":50},"in_reply_to":"ff570b3c_646e4197","updated":"2020-06-16 02:27:58.000000000","message":"hmm, under the assumption of verification failure is the rare case, I think your way should be fine. Thanks.","commit_id":"3401a2fe402c2e205623221fc6a720f02596237f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"59ff05072a1c1783101f05a5a3b48a11e2daf77c","unresolved":false,"context_lines":[{"line_number":382,"context_line":"        return self._verify_and_write(context, image_id, trusted_certs,"},{"line_number":383,"context_line":"                                      image_chunks, data, dst_path, True)"},{"line_number":384,"context_line":""},{"line_number":385,"context_line":"    def _verify_and_write(self, context, image_id, trusted_certs,"},{"line_number":386,"context_line":"                          image_chunks, data, dst_path, write_image):"},{"line_number":387,"context_line":"        \"\"\"Perform image signature verification and save the image file if needed."},{"line_number":388,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"bf51134e_8101353b","line":385,"updated":"2020-06-25 13:24:53.000000000","message":"Please do the extract method refactoring in a separate patch. That would help reviewing the change significantly.","commit_id":"45613120a4ee3f406c47f99b5796720b3e3a8d50"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"59ff05072a1c1783101f05a5a3b48a11e2daf77c","unresolved":false,"context_lines":[{"line_number":393,"context_line":"        :param data: File object to use when writing the image."},{"line_number":394,"context_line":"            If passed as None and dst_path is provided, new file is opened."},{"line_number":395,"context_line":"        :param write_image: Write the image file provided by image_chunks."},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"        \"\"\""},{"line_number":398,"context_line":""},{"line_number":399,"context_line":"        close_file \u003d False"}],"source_content_type":"text/x-python","patch_set":28,"id":"bf51134e_416f1dfb","line":396,"updated":"2020-06-25 13:24:53.000000000","message":"please specify the return value of the function","commit_id":"45613120a4ee3f406c47f99b5796720b3e3a8d50"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3f5c66c889f201f451df2f42b16a646ba93a0db2","unresolved":false,"context_lines":[{"line_number":48,"context_line":"from nova import profiler"},{"line_number":49,"context_line":"from nova import service_auth"},{"line_number":50,"context_line":"from nova import utils"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":54,"context_line":"CONF \u003d nova.conf.CONF"}],"source_content_type":"text/x-python","patch_set":29,"id":"bf51134e_56862815","side":"PARENT","line":51,"updated":"2020-07-23 09:07:30.000000000","message":"unrelated change","commit_id":"aaef6ced1079fa12ded9ae1b4bf0f7991b069599"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3f5c66c889f201f451df2f42b16a646ba93a0db2","unresolved":false,"context_lines":[{"line_number":349,"context_line":"        :returns an iterable with image data, or nothing. Iterable is returned"},{"line_number":350,"context_line":"            only when data param is None and dst_path is not provided (assuming"},{"line_number":351,"context_line":"            the caller wants to process the data by itself)."},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"        \"\"\""},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        close_file \u003d False"}],"source_content_type":"text/x-python","patch_set":29,"id":"bf51134e_b6f8647c","side":"PARENT","line":352,"updated":"2020-07-23 09:07:30.000000000","message":"unrealated","commit_id":"aaef6ced1079fa12ded9ae1b4bf0f7991b069599"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3f5c66c889f201f451df2f42b16a646ba93a0db2","unresolved":false,"context_lines":[{"line_number":395,"context_line":"            else:"},{"line_number":396,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":397,"context_line":"                    LOG.error(\"Error during image verification: %s\", ex)"},{"line_number":398,"context_line":""},{"line_number":399,"context_line":"        finally:"},{"line_number":400,"context_line":"            if close_file:"},{"line_number":401,"context_line":"                # Ensure that the data is pushed all the way down to"}],"source_content_type":"text/x-python","patch_set":29,"id":"bf51134e_96f3a09f","side":"PARENT","line":398,"updated":"2020-07-23 09:07:30.000000000","message":"unrelated","commit_id":"aaef6ced1079fa12ded9ae1b4bf0f7991b069599"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"09a8b7897d0600e6289f9eab5518afd00d5486d4","unresolved":false,"context_lines":[{"line_number":236,"context_line":"        \"\"\""},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        # avoid circular import"},{"line_number":239,"context_line":"        from nova.virt.libvirt.storage import rbd_utils"},{"line_number":240,"context_line":"        try:"},{"line_number":241,"context_line":"            # Parse the RBD URL from url_parts, it should consist of 4"},{"line_number":242,"context_line":"            # sections and be in the format of:"}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_41814a4a","line":239,"range":{"start_line":239,"start_character":8,"end_line":239,"end_character":55},"updated":"2020-08-18 19:57:49.000000000","message":"nit - Could use a TODO here to refactor this module into a more generic part of the codebase, feels odd to be importing it directly into nova.image.glance tbh.","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bc815eca732535bde78eebb8c40bd15382a3380a","unresolved":false,"context_lines":[{"line_number":236,"context_line":"        \"\"\""},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        # avoid circular import"},{"line_number":239,"context_line":"        from nova.virt.libvirt.storage import rbd_utils"},{"line_number":240,"context_line":"        try:"},{"line_number":241,"context_line":"            # Parse the RBD URL from url_parts, it should consist of 4"},{"line_number":242,"context_line":"            # sections and be in the format of:"}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_3d3b8eeb","line":239,"range":{"start_line":239,"start_character":8,"end_line":239,"end_character":55},"in_reply_to":"9f560f44_41814a4a","updated":"2020-08-19 10:47:11.000000000","message":"This should be done before the change, IMO, particularly given the need to move the conf options as you\u0027ve suggested below","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"09a8b7897d0600e6289f9eab5518afd00d5486d4","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            LOG.error(msg)"},{"line_number":250,"context_line":"            raise nova.exception.InvalidParameterValue(msg)"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver()"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"        try:"},{"line_number":255,"context_line":"            LOG.debug(\"Attempting to export RBD image: \""}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_e1021eb4","line":252,"range":{"start_line":252,"start_character":8,"end_line":252,"end_character":42},"updated":"2020-08-18 19:57:49.000000000","message":"https://github.com/openstack/nova/blob/cff7382fb3de4eef7aeddc34b6ae3409cbe50ea0/nova/virt/libvirt/storage/rbd_utils.py#L128-L131\n\nSo either the operator needs to configure [libvirt]/rbd_user and [libvirt]/rbd_ceph_conf or rely on the default /etc/ceph/ceph.conf providing everything right?\n\nFeels like we should call that out somewhere, a FUP for this could be to also add separate configurables outside of [libvirt] for the user, conf path etc.","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"df27a438e7c4be119223866b52a2eb9d681ca5b4","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            LOG.error(msg)"},{"line_number":250,"context_line":"            raise nova.exception.InvalidParameterValue(msg)"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver()"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"        try:"},{"line_number":255,"context_line":"            LOG.debug(\"Attempting to export RBD image: \""}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_b3b34f4d","line":252,"range":{"start_line":252,"start_character":8,"end_line":252,"end_character":42},"in_reply_to":"9f560f44_9d29fa9c","updated":"2020-08-19 12:17:44.000000000","message":"Yeah it\u0027s usable outside of the libvirt driver so I\u0027d rather go with generic config opts here.","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bc815eca732535bde78eebb8c40bd15382a3380a","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            LOG.error(msg)"},{"line_number":250,"context_line":"            raise nova.exception.InvalidParameterValue(msg)"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"        rbd_driver \u003d rbd_utils.RBDDriver()"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"        try:"},{"line_number":255,"context_line":"            LOG.debug(\"Attempting to export RBD image: \""}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_9d29fa9c","line":252,"range":{"start_line":252,"start_character":8,"end_line":252,"end_character":42},"in_reply_to":"9f560f44_e1021eb4","updated":"2020-08-19 10:47:11.000000000","message":"Agreed. If this is being done generically as opposed to on a per-driver basis, then Lee is correct in suggesting these should be generic config opts. An alternative approach is to  instead allow overriding of direct download methods on a per-(virt) driver basis. Perhaps that would make more sense, assuming this won\u0027t work for Xen, HyperV, VMWare, etc.?","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bc815eca732535bde78eebb8c40bd15382a3380a","unresolved":false,"context_lines":[{"line_number":357,"context_line":""},{"line_number":358,"context_line":"                        # Load chunks from the downloaded image file"},{"line_number":359,"context_line":"                        # for verification (if required)"},{"line_number":360,"context_line":"                        data \u003d open(dst_path, \u0027rb\u0027)"},{"line_number":361,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":362,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":363,"context_line":"                            downloaded_length)"}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_3db8ae74","line":360,"range":{"start_line":360,"start_character":0,"end_line":360,"end_character":51},"updated":"2020-08-19 10:47:11.000000000","message":"This leaves an open file handle, right? Any reason not to use the context manager approach:\n\n  with open(dst_path, \u0027rb\u0027) as fh:\n      ...","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"1d15faa2357f36b274cf088549af539cdf883a7f","unresolved":false,"context_lines":[{"line_number":256,"context_line":"            connect_timeout\u003dCONF.glance.rbd_connect_timeout)"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"        try:"},{"line_number":259,"context_line":"            LOG.debug(f\"Attempting to export RBD image: \""},{"line_number":260,"context_line":"                      \"{pool}/{image_uuid}/{snapshot_name} to {dst_path}\")"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"            rbd_driver.export_image(dst_path, image_uuid,"}],"source_content_type":"text/x-python","patch_set":32,"id":"9f560f44_69670853","line":259,"updated":"2020-08-19 14:00:09.000000000","message":"pep8: F541 f-string is missing placeholders","commit_id":"a455f1010e26d8563844947d1eaba6837adc6848"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b87d6cc4959d6bbf7a975bfd4f2d0bf0a0dc9ede","unresolved":false,"context_lines":[{"line_number":256,"context_line":"            connect_timeout\u003dCONF.glance.rbd_connect_timeout)"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"        try:"},{"line_number":259,"context_line":"            LOG.debug(f\"Attempting to export RBD image: \""},{"line_number":260,"context_line":"                      \"{pool}/{image_uuid}/{snapshot_name} to {dst_path}\")"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"            rbd_driver.export_image(dst_path, image_uuid,"}],"source_content_type":"text/x-python","patch_set":32,"id":"9f560f44_a1765405","line":259,"in_reply_to":"9f560f44_69670853","updated":"2020-08-19 17:23:10.000000000","message":"I usually stick f in front of every line when I want multi-line f-strings. With that said, I don\u0027t think you actually want to use this here since logging can use delayed interpolation, right?","commit_id":"a455f1010e26d8563844947d1eaba6837adc6848"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d076bec3cd33e37773a7380603ebb7f9791fa266","unresolved":false,"context_lines":[{"line_number":256,"context_line":"            connect_timeout\u003dCONF.glance.rbd_connect_timeout)"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"        try:"},{"line_number":259,"context_line":"            LOG.debug(f\"Attempting to export RBD image: \""},{"line_number":260,"context_line":"                      \"{pool}/{image_uuid}/{snapshot_name} to {dst_path}\")"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"            rbd_driver.export_image(dst_path, image_uuid,"}],"source_content_type":"text/x-python","patch_set":32,"id":"9f560f44_9c48d3cc","line":259,"in_reply_to":"9f560f44_a1765405","updated":"2020-08-19 18:14:59.000000000","message":"Yeah I\u0027ll revert this back my bad.","commit_id":"a455f1010e26d8563844947d1eaba6837adc6848"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"cf31b87e7a9dbe5ed80b5680b0c33ebb9903e37a","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        # to be added here."},{"line_number":223,"context_line":"        self._download_handlers \u003d {}"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        if \u0027rbd\u0027 in CONF.glance.allowed_direct_url_schemes:"},{"line_number":226,"context_line":"            self._download_handlers[\u0027rbd\u0027] \u003d self.rbd_download"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"    def rbd_download(self, context, url_parts, dst_path, metadata\u003dNone):"}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_22a30bb4","line":225,"range":{"start_line":225,"start_character":20,"end_line":225,"end_character":59},"updated":"2020-08-25 15:42:05.000000000","message":"As a general design point, do we ever intend to expand this list further (like, in the next year or two - a decade from now doesn\u0027t count :P). If not, could we maintain the deprecation and simply introduce a new option? \u0027[glance] enable_rbd_download\u0027 (or something less wordy)","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f960f28dacbe5127e8046107cb37a9268a0de5b8","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            cluster_uuid, pool_name, image_uuid, snapshot_name \u003d ("},{"line_number":246,"context_line":"                url_path.split(\u0027/\u0027))"},{"line_number":247,"context_line":"        except ValueError as e:"},{"line_number":248,"context_line":"            msg \u003d f\"Invalid RBD URL format: {e}\""},{"line_number":249,"context_line":"            LOG.error(msg)"},{"line_number":250,"context_line":"            raise nova.exception.InvalidParameterValue(msg)"},{"line_number":251,"context_line":""}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_a299dbc4","line":248,"range":{"start_line":248,"start_character":12,"end_line":248,"end_character":48},"updated":"2020-08-25 15:34:43.000000000","message":"Doesn\u0027t this need to be translated?","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f960f28dacbe5127e8046107cb37a9268a0de5b8","unresolved":false,"context_lines":[{"line_number":256,"context_line":"            connect_timeout\u003dCONF.glance.rbd_connect_timeout)"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"        try:"},{"line_number":259,"context_line":"            LOG.debug(\"Attempting to export RBD image: \""},{"line_number":260,"context_line":"                      \"[pool_name: %s] [image_uuid: %s] \""},{"line_number":261,"context_line":"                      \"[snapshot_name: %s] [dst_path: %s]\","},{"line_number":262,"context_line":"                      pool_name, image_uuid, snapshot_name, dst_path)"}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_c2dfaf63","line":259,"range":{"start_line":259,"start_character":22,"end_line":259,"end_character":23},"updated":"2020-08-25 15:34:43.000000000","message":"style nit: would be nice if you dragged this down to the next line\n\n  LOG.debug(\n      \"Attempting to export RBD image: \"\n      ...)","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f960f28dacbe5127e8046107cb37a9268a0de5b8","unresolved":false,"context_lines":[{"line_number":261,"context_line":"                      \"[snapshot_name: %s] [dst_path: %s]\","},{"line_number":262,"context_line":"                      pool_name, image_uuid, snapshot_name, dst_path)"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"            rbd_driver.export_image(dst_path, image_uuid,"},{"line_number":265,"context_line":"                                    snapshot_name, pool_name)"},{"line_number":266,"context_line":"        except Exception as e:"},{"line_number":267,"context_line":"            LOG.error(\"Error during RBD image export: %s\", e)"}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_62ca23a1","line":264,"updated":"2020-08-25 15:34:43.000000000","message":"ditto (hanging indent)","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f960f28dacbe5127e8046107cb37a9268a0de5b8","unresolved":false,"context_lines":[{"line_number":361,"context_line":""},{"line_number":362,"context_line":"                        # Load chunks from the downloaded image file"},{"line_number":363,"context_line":"                        # for verification (if required)"},{"line_number":364,"context_line":"                        data \u003d open(dst_path, \u0027rb\u0027)"},{"line_number":365,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":366,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":367,"context_line":"                            downloaded_length)"}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_22eb0b13","line":364,"range":{"start_line":364,"start_character":24,"end_line":364,"end_character":51},"updated":"2020-08-25 15:34:43.000000000","message":"Doesn\u0027t this leave an open file handle? Asked before [1]\n\n[1] https://review.opendev.org/#/c/574301/31/nova/image/glance.py@360","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"18ae6fd4b9b96645f053688e07921681ae1e9f26","unresolved":false,"context_lines":[{"line_number":361,"context_line":""},{"line_number":362,"context_line":"                        # Load chunks from the downloaded image file"},{"line_number":363,"context_line":"                        # for verification (if required)"},{"line_number":364,"context_line":"                        data \u003d open(dst_path, \u0027rb\u0027)"},{"line_number":365,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":366,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":367,"context_line":"                            downloaded_length)"}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_4201df69","line":364,"range":{"start_line":364,"start_character":24,"end_line":364,"end_character":51},"in_reply_to":"9f560f44_22eb0b13","updated":"2020-08-25 15:56:33.000000000","message":"OK","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b0bb266e73dc4492e6b45fea61c1ab6ff30c5cdc","unresolved":false,"context_lines":[{"line_number":361,"context_line":""},{"line_number":362,"context_line":"                        # Load chunks from the downloaded image file"},{"line_number":363,"context_line":"                        # for verification (if required)"},{"line_number":364,"context_line":"                        data \u003d open(dst_path, \u0027rb\u0027)"},{"line_number":365,"context_line":"                        downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":366,"context_line":"                        image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":367,"context_line":"                            downloaded_length)"}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_c2484fb4","line":364,"range":{"start_line":364,"start_character":24,"end_line":364,"end_character":51},"in_reply_to":"9f560f44_22eb0b13","updated":"2020-08-25 16:01:16.000000000","message":"the IterableWithLength has a finally block with a close() call but I\u0027m not 100% sure","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"aecf9bce837f5299d932266a60d9324d61a25d8e","unresolved":false,"context_lines":[{"line_number":363,"context_line":"                        # for verification (if required)"},{"line_number":364,"context_line":"                        with open(dst_path, \u0027rb\u0027) as data:"},{"line_number":365,"context_line":"                            downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":366,"context_line":"                            image_chunks \u003d glance_utils.IterableWithLength(data,"},{"line_number":367,"context_line":"                                downloaded_length)"},{"line_number":368,"context_line":"                            self._verify_and_write(context, image_id,"},{"line_number":369,"context_line":"                                trusted_certs, image_chunks, None, None)"}],"source_content_type":"text/x-python","patch_set":36,"id":"9f560f44_47756c63","line":366,"updated":"2020-08-25 19:31:51.000000000","message":"pep8: E501 line too long (80 \u003e 79 characters)","commit_id":"ed996a407ad69ab57c2b2b0e5e9941c5af3974d4"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"6d71fce001257c702c02a39fd5e2d8b3f28b9ab0","unresolved":false,"context_lines":[{"line_number":307,"context_line":"    def _get_transfer_method(self, scheme):"},{"line_number":308,"context_line":"        \"\"\"Returns a transfer method for scheme, or None.\"\"\""},{"line_number":309,"context_line":"        try:"},{"line_number":310,"context_line":"            return self._download_handlers[scheme]"},{"line_number":311,"context_line":"        except KeyError:"},{"line_number":312,"context_line":"            return None"},{"line_number":313,"context_line":""}],"source_content_type":"text/x-python","patch_set":40,"id":"9f560f44_c9284ae3","line":310,"updated":"2020-08-27 14:49:14.000000000","message":"This bit already exists, we\u0027re just adding the \u0027rbd\u0027 scheme on L225.","commit_id":"3253c743cd487b443aac0572a65b9b4979a53158"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"6d71fce001257c702c02a39fd5e2d8b3f28b9ab0","unresolved":false,"context_lines":[{"line_number":360,"context_line":"                        LOG.info(\"Successfully transferred using %s\", o.scheme)"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"                        # Load chunks from the downloaded image file"},{"line_number":363,"context_line":"                        # for verification (if required)"},{"line_number":364,"context_line":"                        with open(dst_path, \u0027rb\u0027) as fh:"},{"line_number":365,"context_line":"                            downloaded_length \u003d os.path.getsize(dst_path)"},{"line_number":366,"context_line":"                            image_chunks \u003d glance_utils.IterableWithLength(fh,"}],"source_content_type":"text/x-python","patch_set":40,"id":"9f560f44_097402fd","line":363,"range":{"start_line":363,"start_character":43,"end_line":363,"end_character":56},"updated":"2020-08-27 14:49:14.000000000","message":"nit: this bit happens inside _verify_and_write(), so it\u0027s kinda misleading to have it in a comment here...","commit_id":"3253c743cd487b443aac0572a65b9b4979a53158"}],"nova/storage/rbd_utils.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f960f28dacbe5127e8046107cb37a9268a0de5b8","unresolved":false,"context_lines":[{"line_number":123,"context_line":"class RBDDriver(object):"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    def __init__(self, pool\u003dNone, user\u003dNone, ceph_conf\u003dNone,"},{"line_number":126,"context_line":"                 connect_timeout\u003dNone):"},{"line_number":127,"context_line":"        if rbd is None:"},{"line_number":128,"context_line":"            raise RuntimeError(_(\u0027rbd python libraries not found\u0027))"},{"line_number":129,"context_line":""}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_a2c71b96","line":126,"updated":"2020-08-25 15:34:43.000000000","message":"A docstring would be super","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"443a180a5c9d608595283e9bbe0f99c365cfd173","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        if rbd is None:"},{"line_number":128,"context_line":"            raise RuntimeError(_(\u0027rbd python libraries not found\u0027))"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        self.pool \u003d pool or CONF.libvirt.images_rbd_pool"},{"line_number":131,"context_line":"        self.rbd_user \u003d user or CONF.libvirt.rbd_user"},{"line_number":132,"context_line":"        self.rbd_connect_timeout \u003d ("},{"line_number":133,"context_line":"            connect_timeout or CONF.libvirt.rbd_connect_timeout)"},{"line_number":134,"context_line":"        self.ceph_conf \u003d ceph_conf or CONF.libvirt.images_rbd_ceph_conf"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def _connect_to_rados(self, pool\u003dNone):"},{"line_number":137,"context_line":"        client \u003d rados.Rados(rados_id\u003dself.rbd_user,"}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_c2668fbb","line":134,"range":{"start_line":130,"start_character":0,"end_line":134,"end_character":71},"updated":"2020-08-25 15:38:52.000000000","message":"Are we deprecating these options going forward? I assume not, given you\u0027ve namespaced the new options in \u0027glance\u0027 and this isn\u0027t glance specific in the libvirt context?\n\nIf not, we probably don\u0027t want to define these here given this is supposed to be a generic namespace? Could we make the four args to this init function mandatory and pass these in any callers in libvirt? I only see four instances. A FUP is fine","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"}],"nova/tests/unit/image/test_glance.py":[{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"bc8d638c9d647abfcac11b91d07e040e358f1871","unresolved":false,"context_lines":[{"line_number":723,"context_line":"        show_mock.return_value \u003d {"},{"line_number":724,"context_line":"            \u0027locations\u0027: ["},{"line_number":725,"context_line":"                {"},{"line_number":726,"context_line":"                    \u0027url\u0027: \u0027rbd://cluster_uuid/pool_name/image_uuid/snapshot_name\u0027,"},{"line_number":727,"context_line":"                    \u0027metadata\u0027: mock.sentinel.loc_meta"},{"line_number":728,"context_line":"                }"},{"line_number":729,"context_line":"            ]"}],"source_content_type":"text/x-python","patch_set":8,"id":"1f493fa4_4c0da68e","line":726,"updated":"2020-04-29 14:11:23.000000000","message":"pep8: E501 line too long (83 \u003e 79 characters)","commit_id":"4046906700f434b941e861558d9cfea156c10729"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3eb2424ea12d34f7101b4b4e6bef9c8401b79b27","unresolved":false,"context_lines":[{"line_number":764,"context_line":"                                         mock.sentinel.dst_path,"},{"line_number":765,"context_line":"                                         mock.sentinel.loc_meta)"},{"line_number":766,"context_line":"        writer.write.assert_not_called()"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageServiceV2._get_transfer_method\u0027)"},{"line_number":769,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageServiceV2.show\u0027)"},{"line_number":770,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageServiceV2._safe_fsync\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"ff570b3c_d821fbfd","line":767,"updated":"2020-05-12 14:42:09.000000000","message":"You need to assert that the regular-path code to open and verify the image is not called, as well as assert that the result of your override code is what gets passed into the image verification bit.","commit_id":"2aca0016075309adc6d478c7f255661648b1756a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bc815eca732535bde78eebb8c40bd15382a3380a","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from oslo_utils.fixture import uuidsentinel as uuids"},{"line_number":29,"context_line":"import six"},{"line_number":30,"context_line":"from six.moves import StringIO"},{"line_number":31,"context_line":"import six.moves.urllib.parse as urlparse"},{"line_number":32,"context_line":"import testtools"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"import nova.conf"}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_1dbb2a72","line":31,"range":{"start_line":31,"start_character":0,"end_line":31,"end_character":41},"updated":"2020-08-19 10:47:11.000000000","message":"This isn\u0027t being backported. Let\u0027s not use six here","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"df27a438e7c4be119223866b52a2eb9d681ca5b4","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from oslo_utils.fixture import uuidsentinel as uuids"},{"line_number":29,"context_line":"import six"},{"line_number":30,"context_line":"from six.moves import StringIO"},{"line_number":31,"context_line":"import six.moves.urllib.parse as urlparse"},{"line_number":32,"context_line":"import testtools"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"import nova.conf"}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_33cf3fb1","line":31,"range":{"start_line":31,"start_character":0,"end_line":31,"end_character":41},"in_reply_to":"9f560f44_1dbb2a72","updated":"2020-08-19 12:17:44.000000000","message":"Done","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bc815eca732535bde78eebb8c40bd15382a3380a","unresolved":false,"context_lines":[{"line_number":689,"context_line":"            service.download(ctx, mock.sentinel.image_id)"},{"line_number":690,"context_line":""},{"line_number":691,"context_line":"    @mock.patch(\u0027os.path.getsize\u0027, return_value\u003d1)"},{"line_number":692,"context_line":"    @mock.patch.object(six.moves.builtins, \u0027open\u0027)"},{"line_number":693,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageServiceV2._get_transfer_method\u0027)"},{"line_number":694,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageServiceV2.show\u0027)"},{"line_number":695,"context_line":"    def test_download_direct_file_uri_v2("}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_7db1868f","line":692,"range":{"start_line":692,"start_character":23,"end_line":692,"end_character":49},"updated":"2020-08-19 10:47:11.000000000","message":"ditto","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bc815eca732535bde78eebb8c40bd15382a3380a","unresolved":false,"context_lines":[{"line_number":693,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageServiceV2._get_transfer_method\u0027)"},{"line_number":694,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageServiceV2.show\u0027)"},{"line_number":695,"context_line":"    def test_download_direct_file_uri_v2("},{"line_number":696,"context_line":"            self, show_mock, get_tran_mock, open_mock, getsize_mock):"},{"line_number":697,"context_line":"        self.flags(allowed_direct_url_schemes\u003d[\u0027file\u0027], group\u003d\u0027glance\u0027)"},{"line_number":698,"context_line":"        show_mock.return_value \u003d {"},{"line_number":699,"context_line":"            \u0027locations\u0027: ["}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_bdab1e1b","line":696,"range":{"start_line":696,"start_character":8,"end_line":696,"end_character":69},"updated":"2020-08-19 10:47:11.000000000","message":"style nit:\n\n  def test_download_direct_file_uri_v2(\n      self, show_mock, get_tran_mock, open_mock, getsize_mock,\n  ):","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bc815eca732535bde78eebb8c40bd15382a3380a","unresolved":false,"context_lines":[{"line_number":1341,"context_line":"        self.pool_name \u003d \"images\""},{"line_number":1342,"context_line":"        self.snapshot_name \u003d \"snap\""},{"line_number":1343,"context_line":""},{"line_number":1344,"context_line":"    \"\"\"Tests the rbd_download method of the GlanceImageServiceV2.\"\"\""},{"line_number":1345,"context_line":"    @mock.patch.object(rbd_utils.RBDDriver, \u0027export_image\u0027)"},{"line_number":1346,"context_line":"    @mock.patch.object(rbd_utils, \u0027rbd\u0027)"},{"line_number":1347,"context_line":"    def test_rbd_download_success(self, mock_rbd, mock_export_image):"}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_dd65321f","line":1344,"range":{"start_line":1344,"start_character":0,"end_line":1344,"end_character":68},"updated":"2020-08-19 10:47:11.000000000","message":"Instead of doing this here, can you add a docstring to each tests outlining what you\u0027re trying to accomplish at a high level or the rationale for the test","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bc815eca732535bde78eebb8c40bd15382a3380a","unresolved":false,"context_lines":[{"line_number":1351,"context_line":""},{"line_number":1352,"context_line":"        service.rbd_download(ctx, self.url_parts, mock.sentinel.dst_path)"},{"line_number":1353,"context_line":""},{"line_number":1354,"context_line":"        mock_export_image.assert_called_once_with(mock.sentinel.dst_path,"},{"line_number":1355,"context_line":"            self.image_uuid,"},{"line_number":1356,"context_line":"            self.snapshot_name,"},{"line_number":1357,"context_line":"            self.pool_name)"}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_3d6d4e04","line":1354,"range":{"start_line":1354,"start_character":50,"end_line":1354,"end_character":73},"updated":"2020-08-19 10:47:11.000000000","message":"nit: can you move this down a line","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bc815eca732535bde78eebb8c40bd15382a3380a","unresolved":false,"context_lines":[{"line_number":1364,"context_line":"        wrong_url \u003d \"http://www.example.com\""},{"line_number":1365,"context_line":"        wrong_url_parts \u003d urlparse.urlparse(wrong_url)"},{"line_number":1366,"context_line":""},{"line_number":1367,"context_line":"        self.assertRaises(exception.InvalidParameterValue,"},{"line_number":1368,"context_line":"                          service.rbd_download,"},{"line_number":1369,"context_line":"                          ctx,"},{"line_number":1370,"context_line":"                          wrong_url_parts,"},{"line_number":1371,"context_line":"                          mock.sentinel.dst_path)"},{"line_number":1372,"context_line":""},{"line_number":1373,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.storage.rbd_utils.RBDDriver.export_image\u0027)"},{"line_number":1374,"context_line":"    @mock.patch.object(rbd_utils, \u0027rbd\u0027)"}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_bd803e8d","line":1371,"range":{"start_line":1367,"start_character":0,"end_line":1371,"end_character":49},"updated":"2020-08-19 10:47:11.000000000","message":"style nit:\n\n  self.assertRaises(\n      exception.InvalidParameterValue,\n      service.rbd_download,\n      ctx,\n      wrong_url_parts,\n      mock.sentinel.dst_path)\n\nbelow too","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f960f28dacbe5127e8046107cb37a9268a0de5b8","unresolved":false,"context_lines":[{"line_number":689,"context_line":"            service.download(ctx, mock.sentinel.image_id)"},{"line_number":690,"context_line":""},{"line_number":691,"context_line":"    @mock.patch(\u0027os.path.getsize\u0027, return_value\u003d1)"},{"line_number":692,"context_line":"    @mock.patch.object(six.moves.builtins, \u0027open\u0027)"},{"line_number":693,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageServiceV2._get_transfer_method\u0027)"},{"line_number":694,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageServiceV2.show\u0027)"},{"line_number":695,"context_line":"    def test_download_direct_file_uri_v2("}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_c2b84f12","line":692,"range":{"start_line":692,"start_character":23,"end_line":692,"end_character":41},"updated":"2020-08-25 15:34:43.000000000","message":"no need to use six nowadays","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f960f28dacbe5127e8046107cb37a9268a0de5b8","unresolved":false,"context_lines":[{"line_number":754,"context_line":"        verifier \u003d mock.MagicMock()"},{"line_number":755,"context_line":"        get_verifier_mock.return_value \u003d verifier"},{"line_number":756,"context_line":""},{"line_number":757,"context_line":"        res \u003d service.download(ctx, mock.sentinel.image_id,"},{"line_number":758,"context_line":"                               dst_path\u003dmock.sentinel.dst_path,"},{"line_number":759,"context_line":"                               trusted_certs\u003dmock.sentinel.trusted_certs)"},{"line_number":760,"context_line":""}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_a24c3b42","line":757,"range":{"start_line":757,"start_character":31,"end_line":757,"end_character":33},"updated":"2020-08-25 15:34:43.000000000","message":"nit: If you could drag this down to the next line, that\u0027d be great.","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f960f28dacbe5127e8046107cb37a9268a0de5b8","unresolved":false,"context_lines":[{"line_number":773,"context_line":""},{"line_number":774,"context_line":"        # verifier called with the value we got from rbd download"},{"line_number":775,"context_line":"        verifier.update.assert_has_calls("},{"line_number":776,"context_line":"                ["},{"line_number":777,"context_line":"                    mock.call(\"rbd1\"),"},{"line_number":778,"context_line":"                    mock.call(\"rbd2\")"},{"line_number":779,"context_line":"                ]"}],"source_content_type":"text/x-python","patch_set":35,"id":"9f560f44_023c0791","line":776,"range":{"start_line":776,"start_character":12,"end_line":776,"end_character":16},"updated":"2020-08-25 15:34:43.000000000","message":"nit","commit_id":"a6dbea62b4598ed0bf01153097bcf620e6fef33b"}],"nova/tests/unit/virt/libvirt/storage/test_rbd.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":644,"context_line":"                                 mock.sentinel.snap,"},{"line_number":645,"context_line":"                                 mock.sentinel.pool)"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"        mock_execute.assert_called_once_with("},{"line_number":648,"context_line":"            \u0027rbd\u0027, \u0027export\u0027,"},{"line_number":649,"context_line":"            \u0027--pool\u0027, mock.sentinel.pool,"},{"line_number":650,"context_line":"            \u0027--image\u0027, mock.sentinel.name,"},{"line_number":651,"context_line":"            \u0027--path\u0027, mock.sentinel.dst_path,"},{"line_number":652,"context_line":"            \u0027--snap\u0027, mock.sentinel.snap)"},{"line_number":653,"context_line":""},{"line_number":654,"context_line":"    @mock.patch(\u0027oslo_concurrency.processutils.execute\u0027)"},{"line_number":655,"context_line":"    def test_export_image_default_pool(self, mock_execute):"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_95c33e57","line":652,"range":{"start_line":647,"start_character":0,"end_line":652,"end_character":41},"updated":"2020-05-19 15:29:10.000000000","message":"Can you use self.flags() to set [libvirt]/rbd_user and [libvirt]/images_rbd_ceph_conf that should then turn up here?","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5083c5a50c2111707b11940e660ef5ce92252bfe","unresolved":false,"context_lines":[{"line_number":644,"context_line":"                                 mock.sentinel.snap,"},{"line_number":645,"context_line":"                                 mock.sentinel.pool)"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"        mock_execute.assert_called_once_with("},{"line_number":648,"context_line":"            \u0027rbd\u0027, \u0027export\u0027,"},{"line_number":649,"context_line":"            \u0027--pool\u0027, mock.sentinel.pool,"},{"line_number":650,"context_line":"            \u0027--image\u0027, mock.sentinel.name,"},{"line_number":651,"context_line":"            \u0027--path\u0027, mock.sentinel.dst_path,"},{"line_number":652,"context_line":"            \u0027--snap\u0027, mock.sentinel.snap)"},{"line_number":653,"context_line":""},{"line_number":654,"context_line":"    @mock.patch(\u0027oslo_concurrency.processutils.execute\u0027)"},{"line_number":655,"context_line":"    def test_export_image_default_pool(self, mock_execute):"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_e7ebd70f","line":652,"range":{"start_line":647,"start_character":0,"end_line":652,"end_character":41},"in_reply_to":"ff570b3c_95c33e57","updated":"2020-05-20 09:45:48.000000000","message":"OK","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bc815eca732535bde78eebb8c40bd15382a3380a","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        self.flags(rbd_connect_timeout\u003dself.rbd_connect_timeout,"},{"line_number":115,"context_line":"                    group\u003d\u0027libvirt\u0027)"},{"line_number":116,"context_line":"        self.flags(images_rbd_ceph_conf\u003d\u0027/foo/bar.conf\u0027, group\u003d\u0027libvirt\u0027)"},{"line_number":117,"context_line":"        self.flags(rbd_user\u003d\u0027foo\u0027, group\u003d\u0027libvirt\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        rados_patcher \u003d mock.patch.object(rbd_utils, \u0027rados\u0027)"},{"line_number":120,"context_line":"        self.mock_rados \u003d rados_patcher.start()"}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_5d5ac24f","line":117,"updated":"2020-08-19 10:47:11.000000000","message":"nit: these can all be done at once, btw:\n\n  self.flags(\n    images_rbd_pool\u003dself.rbd_pool,\n    images_rbd_ceph_conf\u003d\u0027/foo/bar.conf\u0027,\n    rbd_connect_timeout\u003dself.rbd_connect_timeout,\n    rbd_user\u003d\u0027foo\u0027,\n    group\u003d\u0027libvirt\u0027)","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"df27a438e7c4be119223866b52a2eb9d681ca5b4","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        self.flags(rbd_connect_timeout\u003dself.rbd_connect_timeout,"},{"line_number":115,"context_line":"                    group\u003d\u0027libvirt\u0027)"},{"line_number":116,"context_line":"        self.flags(images_rbd_ceph_conf\u003d\u0027/foo/bar.conf\u0027, group\u003d\u0027libvirt\u0027)"},{"line_number":117,"context_line":"        self.flags(rbd_user\u003d\u0027foo\u0027, group\u003d\u0027libvirt\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        rados_patcher \u003d mock.patch.object(rbd_utils, \u0027rados\u0027)"},{"line_number":120,"context_line":"        self.mock_rados \u003d rados_patcher.start()"}],"source_content_type":"text/x-python","patch_set":31,"id":"9f560f44_93f8cbc4","line":117,"in_reply_to":"9f560f44_5d5ac24f","updated":"2020-08-19 12:17:44.000000000","message":"Done","commit_id":"a7e3062fd8bf6a9c63b95aada8e0c4ecda1aacf7"}],"nova/virt/libvirt/storage/rbd_utils.py":[{"author":{"_account_id":10338,"name":"Daniel Speichert","email":"daniel@speichert.pl","username":"daniel"},"change_message_id":"c9874d99e823937d4c7151a4e3efbc4e00be055a","unresolved":false,"context_lines":[{"line_number":328,"context_line":"        args +\u003d self.ceph_args()"},{"line_number":329,"context_line":"        processutils.execute(\u0027rbd\u0027, \u0027import\u0027, *args)"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"    def export_image(self, base, name, snap, pool\u003dNone):"},{"line_number":332,"context_line":"        \"\"\"Export RBD volume to image file."},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"        Uses the command line export to export a snapshot to an image."}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_358387cd","line":331,"range":{"start_line":331,"start_character":39,"end_line":331,"end_character":43},"updated":"2018-06-11 15:52:37.000000000","message":"could default to \"snap\"","commit_id":"e8c149a8b8fe57e954088d82583caafd53d2a0b2"},{"author":{"_account_id":20812,"name":"Chris Holcombe","email":"chris.holcombe@canonical.com","username":"xfactor973"},"change_message_id":"e8365e3ffa1f790e0d4446e3af95d509b6bd94b0","unresolved":false,"context_lines":[{"line_number":341,"context_line":"        if pool is None:"},{"line_number":342,"context_line":"            pool \u003d self.pool"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        args \u003d [\u0027--pool\u0027, pool, \u0027--image\u0027, name, \u0027--path\u0027, base, \u0027--snap\u0027, snap]"},{"line_number":345,"context_line":"        args +\u003d self.ceph_args()"},{"line_number":346,"context_line":"        utils.execute(\u0027rbd\u0027, \u0027export\u0027, *args)"},{"line_number":347,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_67f05692","line":344,"updated":"2018-06-12 15:40:18.000000000","message":"Are there any tests in nova that will make sure that this doesn\u0027t break in the future?  Ideally librbd should be used here but we\u0027ve seen at comcast in testing that it\u0027s much slower.  I understand the usage here but ceph makes no claims about their CLI not changing and this introduces another breakage point into the codebase.","commit_id":"da0d9b4b4b84e62e11cbe3bb6f4bcea957a925ae"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"bc8d638c9d647abfcac11b91d07e040e358f1871","unresolved":false,"context_lines":[{"line_number":34,"context_line":"import nova.conf"},{"line_number":35,"context_line":"from nova import exception"},{"line_number":36,"context_line":"from nova import utils"},{"line_number":37,"context_line":"from nova.i18n import _"},{"line_number":38,"context_line":"from nova.virt.libvirt import utils as libvirt_utils"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1f493fa4_cc47f6bb","line":37,"updated":"2020-04-29 14:11:23.000000000","message":"pep8: H306: imports not in alphabetical order (nova.utils, nova.i18n._)","commit_id":"4046906700f434b941e861558d9cfea156c10729"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"bc8d638c9d647abfcac11b91d07e040e358f1871","unresolved":false,"context_lines":[{"line_number":348,"context_line":"        if pool is None:"},{"line_number":349,"context_line":"            pool \u003d self.pool"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        args \u003d [\u0027--pool\u0027, pool, \u0027--image\u0027, name, \u0027--path\u0027, base, \u0027--snap\u0027, snap]"},{"line_number":352,"context_line":"        args +\u003d self.ceph_args()"},{"line_number":353,"context_line":"        utils.execute(\u0027rbd\u0027, \u0027export\u0027, *args)"},{"line_number":354,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1f493fa4_ac426aa9","line":351,"updated":"2020-04-29 14:11:23.000000000","message":"pep8: E501 line too long (80 \u003e 79 characters)","commit_id":"4046906700f434b941e861558d9cfea156c10729"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3eb2424ea12d34f7101b4b4e6bef9c8401b79b27","unresolved":false,"context_lines":[{"line_number":335,"context_line":"        args +\u003d self.ceph_args()"},{"line_number":336,"context_line":"        processutils.execute(\u0027rbd\u0027, \u0027import\u0027, *args)"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"    def export_image(self, base, name, snap, pool\u003dNone):"},{"line_number":339,"context_line":"        \"\"\"Export RBD volume to image file."},{"line_number":340,"context_line":""},{"line_number":341,"context_line":"        Uses the command line export to export a snapshot to an image."}],"source_content_type":"text/x-python","patch_set":15,"id":"ff570b3c_38059778","line":338,"updated":"2020-05-12 14:42:09.000000000","message":"This needs a test","commit_id":"2aca0016075309adc6d478c7f255661648b1756a"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"647eabc70b900d7c77e18edac9b8b5d81c51e12c","unresolved":false,"context_lines":[{"line_number":34,"context_line":"import nova.conf"},{"line_number":35,"context_line":"from nova import exception"},{"line_number":36,"context_line":"from nova.i18n import _"},{"line_number":37,"context_line":"from nova import utils"},{"line_number":38,"context_line":"from nova.virt.libvirt import utils as libvirt_utils"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"ff570b3c_179afd6c","line":37,"updated":"2020-05-13 18:01:34.000000000","message":"pep8: F401 \u0027nova.utils\u0027 imported but unused","commit_id":"45b4a69895107444ca14cca7f93216998989276b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":337,"context_line":"    def export_image(self, base, name, snap, pool\u003dNone):"},{"line_number":338,"context_line":"        \"\"\"Export RBD volume to image file."},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"        Uses the command line export to export a snapshot to an image."},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        :base: Path to image file"},{"line_number":343,"context_line":"        :name: Name of RBD volume"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_556a4619","line":340,"range":{"start_line":340,"start_character":64,"end_line":340,"end_character":69},"updated":"2020-05-19 15:29:10.000000000","message":"local image file","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":337,"context_line":"    def export_image(self, base, name, snap, pool\u003dNone):"},{"line_number":338,"context_line":"        \"\"\"Export RBD volume to image file."},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"        Uses the command line export to export a snapshot to an image."},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        :base: Path to image file"},{"line_number":343,"context_line":"        :name: Name of RBD volume"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_f57b9af1","line":340,"range":{"start_line":340,"start_character":49,"end_line":340,"end_character":57},"updated":"2020-05-19 15:29:10.000000000","message":"rbd volume snapshot","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        :base: Path to image file"},{"line_number":343,"context_line":"        :name: Name of RBD volume"},{"line_number":344,"context_line":"        :snap: Name of RBD snapshot"},{"line_number":345,"context_line":"        :pool: Name of RBD pool"},{"line_number":346,"context_line":"        \"\"\""},{"line_number":347,"context_line":"        if pool is None:"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_d5cc9636","line":344,"range":{"start_line":344,"start_character":0,"end_line":344,"end_character":35},"updated":"2020-05-19 15:29:10.000000000","message":"I wonder if this should also be optional?","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"4d4724f7c5288bc3732e242d0b41f81f0fcae90c","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        :base: Path to image file"},{"line_number":343,"context_line":"        :name: Name of RBD volume"},{"line_number":344,"context_line":"        :snap: Name of RBD snapshot"},{"line_number":345,"context_line":"        :pool: Name of RBD pool"},{"line_number":346,"context_line":"        \"\"\""},{"line_number":347,"context_line":"        if pool is None:"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_aa01a6b1","line":344,"range":{"start_line":344,"start_character":0,"end_line":344,"end_character":35},"in_reply_to":"ff570b3c_87e01b28","updated":"2020-05-20 10:23:40.000000000","message":"Not in your case so feel free to ignore this.","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"5083c5a50c2111707b11940e660ef5ce92252bfe","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        :base: Path to image file"},{"line_number":343,"context_line":"        :name: Name of RBD volume"},{"line_number":344,"context_line":"        :snap: Name of RBD snapshot"},{"line_number":345,"context_line":"        :pool: Name of RBD pool"},{"line_number":346,"context_line":"        \"\"\""},{"line_number":347,"context_line":"        if pool is None:"}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_87e01b28","line":344,"range":{"start_line":344,"start_character":0,"end_line":344,"end_character":35},"in_reply_to":"ff570b3c_d5cc9636","updated":"2020-05-20 09:45:48.000000000","message":"Could it? Could the direct URL pointing to the image miss it?","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d0a3953a821abc68b0cd0ce7accb0db33fd4b7e0","unresolved":false,"context_lines":[{"line_number":350,"context_line":"        args \u003d [\u0027--pool\u0027, pool, \u0027--image\u0027, name, \u0027--path\u0027, base,"},{"line_number":351,"context_line":"                \u0027--snap\u0027, snap]"},{"line_number":352,"context_line":"        args +\u003d self.ceph_args()"},{"line_number":353,"context_line":"        processutils.execute(\u0027rbd\u0027, \u0027export\u0027, *args)"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"    def _destroy_volume(self, client, volume, pool\u003dNone):"},{"line_number":356,"context_line":"        \"\"\"Destroy an RBD volume, retrying as needed."}],"source_content_type":"text/x-python","patch_set":18,"id":"ff570b3c_b5fc823e","line":353,"range":{"start_line":353,"start_character":0,"end_line":353,"end_character":52},"updated":"2020-05-19 15:29:10.000000000","message":"We should really start moving all of this under privsep in V but that\u0027s unrelated to this change.","commit_id":"9c7044970564837b8bc85678ab744f27815e45b2"}]}