)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f7bd53d7bf9e5a0f53691b68b30e79f9e4ecc815","unresolved":true,"context_lines":[{"line_number":15,"context_line":"the import request comes in, any other host will proxy that HTTP"},{"line_number":16,"context_line":"request direct to the original host instead of trying to do the import"},{"line_number":17,"context_line":"itself."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I12daccb43c535b579c22f9d0742039b2ab42e929"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"f3275606_64641b6f","line":18,"updated":"2021-02-17 14:06:05.000000000","message":"This should tagged with \nImplements: blueprint distributed-image-import","commit_id":"059a161ff080ab1db47c991dcb5ab18cb71f4f1d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b0860915c587895bd422baf86e1f40e5424829be","unresolved":true,"context_lines":[{"line_number":15,"context_line":"the import request comes in, any other host will proxy that HTTP"},{"line_number":16,"context_line":"request direct to the original host instead of trying to do the import"},{"line_number":17,"context_line":"itself."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I12daccb43c535b579c22f9d0742039b2ab42e929"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"6b4fe80a_67c93641","line":18,"in_reply_to":"f3275606_64641b6f","updated":"2021-02-17 14:28:18.000000000","message":"Ack, that didn\u0027t exist when I wrote this :)","commit_id":"059a161ff080ab1db47c991dcb5ab18cb71f4f1d"}],"glance/api/v2/image_data.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"aa0563fcaa1dd4a886fafec58212e22397b66a10","unresolved":true,"context_lines":[{"line_number":337,"context_line":"                \u0027os_glance_staging_store\u0027)"},{"line_number":338,"context_line":"        else:"},{"line_number":339,"context_line":"            staging_store \u003d _build_staging_store()"},{"line_number":340,"context_line":""},{"line_number":341,"context_line":"        try:"},{"line_number":342,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":343,"context_line":"            image.status \u003d \u0027uploading\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"1d109148_9f669a3d","line":340,"updated":"2021-02-01 18:21:47.000000000","message":"AFAIK, this is a refactor, unrelated to the actual change you\u0027re making here. Please split this out to a different patch to keep the functional changes separate from cleanups.","commit_id":"14b15031a9a1aa85ed14c725ebef6d4d52419fcb"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"99a5c8c424d7446bbb1159a8a9adfdf2b4ba1abb","unresolved":false,"context_lines":[{"line_number":337,"context_line":"                \u0027os_glance_staging_store\u0027)"},{"line_number":338,"context_line":"        else:"},{"line_number":339,"context_line":"            staging_store \u003d _build_staging_store()"},{"line_number":340,"context_line":""},{"line_number":341,"context_line":"        try:"},{"line_number":342,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":343,"context_line":"            image.status \u003d \u0027uploading\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"abcaf476_ac5d417c","line":340,"in_reply_to":"1d109148_9f669a3d","updated":"2021-02-01 20:56:32.000000000","message":"Well that\u0027s where you\u0027re wrong. The key is the line 294 of PS10 (I\u0027ll highlight it for you), which will cause us actually to perform and track the staging data a staging data instead of just writing it to the filesystem.","commit_id":"14b15031a9a1aa85ed14c725ebef6d4d52419fcb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3b2f34ab608c0d4ef53b8b7674da219108bfe63a","unresolved":false,"context_lines":[{"line_number":337,"context_line":"                \u0027os_glance_staging_store\u0027)"},{"line_number":338,"context_line":"        else:"},{"line_number":339,"context_line":"            staging_store \u003d _build_staging_store()"},{"line_number":340,"context_line":""},{"line_number":341,"context_line":"        try:"},{"line_number":342,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":343,"context_line":"            image.status \u003d \u0027uploading\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"0812a025_f2d320d3","line":340,"in_reply_to":"abcaf476_ac5d417c","updated":"2021-02-01 22:27:37.000000000","message":"No, I understand that this is a refactor *and* some new functionality, that\u0027s my complaint. In a bisect later where we\u0027re trying to track a regression, the temporal context of what was going on here will be lost and all we will see is two large hunks seemingly move across the change.\n\nIn an attempt to be more clear, let me spell this out in more detail. I meant that the refactoring of this:\n\n if CONF.enabled_backends:\n     # Get the store from the config\n else:\n     # Build the store the \"horrible\" way\n\ninto passing staging\u003dTrue to the set_data() call, is what could and should be split out separately. AFAICT, that is a DRYing up of this hack in more than one place, and stands on its own as using the staging store more like a store and less like a hack. That could be formalized and merged as tech debt reduction *separate* from the new feature of stashing the staging host and proxying to it when appropriate.","commit_id":"14b15031a9a1aa85ed14c725ebef6d4d52419fcb"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"030ef4d47130d30c0b7d960bb88742093b256a09","unresolved":true,"context_lines":[{"line_number":342,"context_line":"            image.status \u003d \u0027uploading\u0027"},{"line_number":343,"context_line":"            image_repo.save(image, from_state\u003d\u0027queued\u0027)"},{"line_number":344,"context_line":"            try:"},{"line_number":345,"context_line":"                staging_store.add("},{"line_number":346,"context_line":"                    image_id, utils.LimitingReader("},{"line_number":347,"context_line":"                        utils.CooperativeReader(data), CONF.image_size_cap), 0)"},{"line_number":348,"context_line":"            except glance_store.Duplicate:"},{"line_number":349,"context_line":"                msg \u003d _(\"The image %s has data on staging\") % image_id"},{"line_number":350,"context_line":"                raise webob.exc.HTTPConflict(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5060153e_c9410054","side":"PARENT","line":347,"range":{"start_line":345,"start_character":15,"end_line":347,"end_character":79},"updated":"2021-02-02 08:22:54.000000000","message":"Is it not possible to to use location url and metadata returned by this add method to insert into locations table instead of modifying set_data method?\n\nThis way we will keep using of reserved staging store, otherwise we don\u0027t have use of this reserved staging store at all.","commit_id":"6558c6e1a0e820c913fd4afacdee14117c8c0c89"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"cb4d44e0bf0992c0d39032cf13590a03f56d06f4","unresolved":true,"context_lines":[{"line_number":342,"context_line":"            image.status \u003d \u0027uploading\u0027"},{"line_number":343,"context_line":"            image_repo.save(image, from_state\u003d\u0027queued\u0027)"},{"line_number":344,"context_line":"            try:"},{"line_number":345,"context_line":"                staging_store.add("},{"line_number":346,"context_line":"                    image_id, utils.LimitingReader("},{"line_number":347,"context_line":"                        utils.CooperativeReader(data), CONF.image_size_cap), 0)"},{"line_number":348,"context_line":"            except glance_store.Duplicate:"},{"line_number":349,"context_line":"                msg \u003d _(\"The image %s has data on staging\") % image_id"},{"line_number":350,"context_line":"                raise webob.exc.HTTPConflict(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":10,"id":"9af6c8c9_fa597a39","side":"PARENT","line":347,"range":{"start_line":345,"start_character":15,"end_line":347,"end_character":79},"in_reply_to":"5060153e_c9410054","updated":"2021-02-03 12:51:31.000000000","message":"But it is using the reserved staging store.","commit_id":"6558c6e1a0e820c913fd4afacdee14117c8c0c89"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"99a5c8c424d7446bbb1159a8a9adfdf2b4ba1abb","unresolved":true,"context_lines":[{"line_number":291,"context_line":"                image.set_data("},{"line_number":292,"context_line":"                    utils.LimitingReader(utils.CooperativeReader(data),"},{"line_number":293,"context_line":"                                         CONF.image_size_cap),"},{"line_number":294,"context_line":"                    staging\u003dTrue)"},{"line_number":295,"context_line":"            except glance_store.Duplicate:"},{"line_number":296,"context_line":"                msg \u003d _(\"The image %s has data on staging\") % image_id"},{"line_number":297,"context_line":"                raise webob.exc.HTTPConflict(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":10,"id":"4460a2a2_17f9ab83","line":294,"updated":"2021-02-01 20:56:32.000000000","message":"It\u0027s this parameter here that will make the locations code to treat this call as staging call instead of creating regular non-staging location.","commit_id":"8ca338089c24f9ec68523657a7d1c7401ab7d17a"}],"glance/api/v2/images.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"babd3c7006ae590883103cd362cc4be3a35342ff","unresolved":true,"context_lines":[{"line_number":213,"context_line":"                       \u0027keys\u0027: \u0027,\u0027.join(changed)})"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def _proxy_import_to_host(self, image, req, body):"},{"line_number":216,"context_line":"        stage_host \u003d image.extra_properties[\u0027os_glance_stage_host\u0027]"},{"line_number":217,"context_line":"        LOG.info(_(\u0027Proxying import request to host %s \u0027"},{"line_number":218,"context_line":"                   \u0027which has image staged\u0027) % stage_host)"},{"line_number":219,"context_line":"        client \u003d glance_context.get_ksa_client(req.context)"}],"source_content_type":"text/x-python","patch_set":2,"id":"95d102e6_e521c645","line":216,"range":{"start_line":216,"start_character":45,"end_line":216,"end_character":65},"updated":"2021-01-11 17:13:59.000000000","message":"just a reminder that this the image owner can modify this between staging and importing, so we\u0027ll require some validation before using this thing.  I wonder whether we need to keep a mapping of \"exposable names\" to \"actual worker direct urls\", and populate the image property with the exposable name.  That won\u0027t eliminate the problem of an end user changing the value of the image property, though.  Maybe the thing to do is to use property protections to make \u0027os_glance_system_*\u0027 CRUD by system admins only, and then use that namespace for all these sensitive image properties, and then it would be perfectly OK to expose the URL, which will be more useful to operators than having an alias in the field.","commit_id":"0f49b81f908f60355e6ac9de6e55e200673488b8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"71ec873b3a95902ee3fedb04b38baaf8799c48f3","unresolved":true,"context_lines":[{"line_number":213,"context_line":"                       \u0027keys\u0027: \u0027,\u0027.join(changed)})"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def _proxy_import_to_host(self, image, req, body):"},{"line_number":216,"context_line":"        stage_host \u003d image.extra_properties[\u0027os_glance_stage_host\u0027]"},{"line_number":217,"context_line":"        LOG.info(_(\u0027Proxying import request to host %s \u0027"},{"line_number":218,"context_line":"                   \u0027which has image staged\u0027) % stage_host)"},{"line_number":219,"context_line":"        client \u003d glance_context.get_ksa_client(req.context)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3dfd7bdb_f9213f8b","line":216,"range":{"start_line":216,"start_character":45,"end_line":216,"end_character":65},"in_reply_to":"42e26e78_53156cbd","updated":"2021-01-11 19:07:07.000000000","message":"Meant to say that I think that codifying os_glance_* as a namespace of internal-only things that can\u0027t be changed externally makes sense. Otherwise we\u0027re just adding them all to the list, and could miss one.","commit_id":"0f49b81f908f60355e6ac9de6e55e200673488b8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c380162712f87246c9f837ca50cb9393d3abb393","unresolved":true,"context_lines":[{"line_number":213,"context_line":"                       \u0027keys\u0027: \u0027,\u0027.join(changed)})"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def _proxy_import_to_host(self, image, req, body):"},{"line_number":216,"context_line":"        stage_host \u003d image.extra_properties[\u0027os_glance_stage_host\u0027]"},{"line_number":217,"context_line":"        LOG.info(_(\u0027Proxying import request to host %s \u0027"},{"line_number":218,"context_line":"                   \u0027which has image staged\u0027) % stage_host)"},{"line_number":219,"context_line":"        client \u003d glance_context.get_ksa_client(req.context)"}],"source_content_type":"text/x-python","patch_set":2,"id":"42e26e78_53156cbd","line":216,"range":{"start_line":216,"start_character":45,"end_line":216,"end_character":65},"in_reply_to":"95d102e6_e521c645","updated":"2021-01-11 18:19:19.000000000","message":"Sorry if it wasn\u0027t clear, but this needs to be a reserved property (like the task lock) and *not* be allowed to change via the API. I don\u0027t even really want to expose it to the user in image show.\n\nThis is set by the stage operation and shouldn\u0027t be change-able any other way.","commit_id":"0f49b81f908f60355e6ac9de6e55e200673488b8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3cddb97f354748d1fe42563e7be7f8270cf6753a","unresolved":true,"context_lines":[{"line_number":287,"context_line":"        return CONF.worker_self_reference_url or CONF.public_endpoint"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    def is_proxyable(self, image):"},{"line_number":290,"context_line":"        \"\"\"Decode if an action is proxyable to a stage host."},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"        If the image has a staging host recorded with a URL that does not match"},{"line_number":293,"context_line":"        ours, then we can proxy our request to that host."}],"source_content_type":"text/x-python","patch_set":8,"id":"3199265b_231a2b53","line":290,"range":{"start_line":290,"start_character":11,"end_line":290,"end_character":17},"updated":"2021-01-15 00:06:18.000000000","message":"Oops, \"decide\"","commit_id":"f5ffbbe90610d8a30f6e471191e932065662ac75"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3b2f34ab608c0d4ef53b8b7674da219108bfe63a","unresolved":true,"context_lines":[{"line_number":1101,"context_line":"            if any(key.startswith(ns) for ns in self._reserved_namespaces):"},{"line_number":1102,"context_line":"                msg \u003d _(\"Attribute \u0027%s\u0027 is reserved.\") % key"},{"line_number":1103,"context_line":"                raise webob.exc.HTTPForbidden(msg)"},{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":"        return dict(image\u003dimage, extra_properties\u003dproperties, tags\u003dtags)"},{"line_number":1106,"context_line":""},{"line_number":1107,"context_line":"    def _get_change_operation_d10(self, raw_change):"}],"source_content_type":"text/x-python","patch_set":10,"id":"8a4e5bbd_84f49400","line":1104,"updated":"2021-02-01 22:27:37.000000000","message":"FWIW, this is rebase noise that would *not* be shown in the diff if you had a patch on top of this one instead of having modified this one and require us to diff PS9..10 in order to see the changes. The longer this goes on, the more of this will show up in diffing PS9 to whatever the final rev is.","commit_id":"8ca338089c24f9ec68523657a7d1c7401ab7d17a"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f7bd53d7bf9e5a0f53691b68b30e79f9e4ecc815","unresolved":true,"context_lines":[{"line_number":253,"context_line":"        \"\"\""},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"        stage_host \u003d image.extra_properties[\u0027os_glance_stage_host\u0027]"},{"line_number":256,"context_line":"        LOG.info(_(\u0027Proxying %s request to host %s \u0027"},{"line_number":257,"context_line":"                   \u0027which has image staged\u0027) % (req.method, stage_host))"},{"line_number":258,"context_line":"        client \u003d glance_context.get_ksa_client(req.context)"},{"line_number":259,"context_line":"        url \u003d \u0027%s%s\u0027 % (stage_host, req.path)"}],"source_content_type":"text/x-python","patch_set":11,"id":"c71393e2_a1fa10d8","line":256,"range":{"start_line":256,"start_character":17,"end_line":256,"end_character":18},"updated":"2021-02-17 14:06:05.000000000","message":"I think we need to use _LI here","commit_id":"059a161ff080ab1db47c991dcb5ab18cb71f4f1d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f7bd53d7bf9e5a0f53691b68b30e79f9e4ecc815","unresolved":true,"context_lines":[{"line_number":263,"context_line":"            r \u003d request_method(url, json\u003dbody, timeout\u003d60)"},{"line_number":264,"context_line":"        except (requests.exceptions.ConnectionError,"},{"line_number":265,"context_line":"                requests.exceptions.ConnectTimeout) as e:"},{"line_number":266,"context_line":"            LOG.error(\u0027Failed to proxy to %r: %s\u0027 % (url, e))"},{"line_number":267,"context_line":"            raise webob.exc.HTTPGatewayTimeout(\u0027Stage host is unavailable\u0027)"},{"line_number":268,"context_line":"        except requests.exceptions.RequestException as e:"},{"line_number":269,"context_line":"            LOG.error(\u0027Failed to proxy to %r: %s\u0027 % (url, e))"}],"source_content_type":"text/x-python","patch_set":11,"id":"8cb88488_7b9bf269","line":266,"range":{"start_line":266,"start_character":22,"end_line":266,"end_character":48},"updated":"2021-02-17 14:06:05.000000000","message":"_LE","commit_id":"059a161ff080ab1db47c991dcb5ab18cb71f4f1d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f7bd53d7bf9e5a0f53691b68b30e79f9e4ecc815","unresolved":true,"context_lines":[{"line_number":266,"context_line":"            LOG.error(\u0027Failed to proxy to %r: %s\u0027 % (url, e))"},{"line_number":267,"context_line":"            raise webob.exc.HTTPGatewayTimeout(\u0027Stage host is unavailable\u0027)"},{"line_number":268,"context_line":"        except requests.exceptions.RequestException as e:"},{"line_number":269,"context_line":"            LOG.error(\u0027Failed to proxy to %r: %s\u0027 % (url, e))"},{"line_number":270,"context_line":"            raise webob.exc.HTTPBadGateway(\u0027Stage host is unavailable\u0027)"},{"line_number":271,"context_line":"        req_id_hdr \u003d \u0027x-openstack-request-id\u0027"},{"line_number":272,"context_line":"        if req_id_hdr in r.headers:"}],"source_content_type":"text/x-python","patch_set":11,"id":"a0656d29_7c807a27","line":269,"range":{"start_line":269,"start_character":22,"end_line":269,"end_character":40},"updated":"2021-02-17 14:06:05.000000000","message":"_LE","commit_id":"059a161ff080ab1db47c991dcb5ab18cb71f4f1d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f7bd53d7bf9e5a0f53691b68b30e79f9e4ecc815","unresolved":true,"context_lines":[{"line_number":278,"context_line":"        return image.image_id"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    @property"},{"line_number":281,"context_line":"    def my_url(self):"},{"line_number":282,"context_line":"        \"\"\"Return the URL we expect to point to us."},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        If this is set to a per-worker URL in worker_self_reference_url,"}],"source_content_type":"text/x-python","patch_set":11,"id":"e39a86c8_2267de55","line":281,"range":{"start_line":281,"start_character":8,"end_line":281,"end_character":14},"updated":"2021-02-17 14:06:05.000000000","message":"Can we use better name here? (like remote_url or proxy_url)","commit_id":"059a161ff080ab1db47c991dcb5ab18cb71f4f1d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b0860915c587895bd422baf86e1f40e5424829be","unresolved":true,"context_lines":[{"line_number":278,"context_line":"        return image.image_id"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    @property"},{"line_number":281,"context_line":"    def my_url(self):"},{"line_number":282,"context_line":"        \"\"\"Return the URL we expect to point to us."},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        If this is set to a per-worker URL in worker_self_reference_url,"}],"source_content_type":"text/x-python","patch_set":11,"id":"8e67c2c6_29e26498","line":281,"range":{"start_line":281,"start_character":8,"end_line":281,"end_character":14},"in_reply_to":"e39a86c8_2267de55","updated":"2021-02-17 14:28:18.000000000","message":"Well, it\u0027s not remote to this worker. What about \"self_url\" ?","commit_id":"059a161ff080ab1db47c991dcb5ab18cb71f4f1d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f7bd53d7bf9e5a0f53691b68b30e79f9e4ecc815","unresolved":true,"context_lines":[{"line_number":711,"context_line":""},{"line_number":712,"context_line":"        image_repo.save(image)"},{"line_number":713,"context_line":""},{"line_number":714,"context_line":"    def _delete_proxy_logic(self, image, req):"},{"line_number":715,"context_line":"        \"\"\"Proxy an image delete to a staging host."},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"        When an image is staged and then deleted, the staging host still"}],"source_content_type":"text/x-python","patch_set":11,"id":"017804be_e16946b2","line":714,"range":{"start_line":714,"start_character":8,"end_line":714,"end_character":27},"updated":"2021-02-17 14:06:05.000000000","message":"should use better name, something like _delete_remote_image?","commit_id":"059a161ff080ab1db47c991dcb5ab18cb71f4f1d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b0860915c587895bd422baf86e1f40e5424829be","unresolved":true,"context_lines":[{"line_number":711,"context_line":""},{"line_number":712,"context_line":"        image_repo.save(image)"},{"line_number":713,"context_line":""},{"line_number":714,"context_line":"    def _delete_proxy_logic(self, image, req):"},{"line_number":715,"context_line":"        \"\"\"Proxy an image delete to a staging host."},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"        When an image is staged and then deleted, the staging host still"}],"source_content_type":"text/x-python","patch_set":11,"id":"2383c8d2_36dcf5ee","line":714,"range":{"start_line":714,"start_character":8,"end_line":714,"end_character":27},"in_reply_to":"017804be_e16946b2","updated":"2021-02-17 14:28:18.000000000","message":"Okay, that doesn\u0027t make it as clear to me that we\u0027re proxying the call directly, but if I can keep the docstring as it is, then that\u0027s fine.","commit_id":"059a161ff080ab1db47c991dcb5ab18cb71f4f1d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"2e16fe18eace96041624e5aee5c261c5168a8d6d","unresolved":true,"context_lines":[{"line_number":254,"context_line":""},{"line_number":255,"context_line":"        stage_host \u003d image.extra_properties[\u0027os_glance_stage_host\u0027]"},{"line_number":256,"context_line":"        LOG.info(_LI(\u0027Proxying %s request to host %s \u0027"},{"line_number":257,"context_line":"                     \u0027which has image staged\u0027) % (req.method, stage_host))"},{"line_number":258,"context_line":"        client \u003d glance_context.get_ksa_client(req.context)"},{"line_number":259,"context_line":"        url \u003d \u0027%s%s\u0027 % (stage_host, req.path)"},{"line_number":260,"context_line":"        req_id_hdr \u003d \u0027x-openstack-request-id\u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"eda54fb0_dc36660c","line":257,"range":{"start_line":257,"start_character":47,"end_line":257,"end_character":48},"updated":"2021-03-01 18:42:23.000000000","message":"should be , instead of %\n(copying sean\u0027s comment form other patch)","commit_id":"bc0525f365c6be078506179b4bafa976ce9c1f0a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"64d672469175282ce28478a8a282daa4d6ff5dfa","unresolved":true,"context_lines":[{"line_number":254,"context_line":""},{"line_number":255,"context_line":"        stage_host \u003d image.extra_properties[\u0027os_glance_stage_host\u0027]"},{"line_number":256,"context_line":"        LOG.info(_LI(\u0027Proxying %s request to host %s \u0027"},{"line_number":257,"context_line":"                     \u0027which has image staged\u0027) % (req.method, stage_host))"},{"line_number":258,"context_line":"        client \u003d glance_context.get_ksa_client(req.context)"},{"line_number":259,"context_line":"        url \u003d \u0027%s%s\u0027 % (stage_host, req.path)"},{"line_number":260,"context_line":"        req_id_hdr \u003d \u0027x-openstack-request-id\u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"7bdbd505_2da16339","line":257,"range":{"start_line":257,"start_character":47,"end_line":257,"end_character":48},"in_reply_to":"eda54fb0_dc36660c","updated":"2021-03-01 18:54:19.000000000","message":"Yep, thanks, I have a real mental block against this rule. Muscle memory I guess.","commit_id":"bc0525f365c6be078506179b4bafa976ce9c1f0a"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"2e16fe18eace96041624e5aee5c261c5168a8d6d","unresolved":true,"context_lines":[{"line_number":263,"context_line":"            r \u003d request_method(url, json\u003dbody, timeout\u003d60)"},{"line_number":264,"context_line":"        except (requests.exceptions.ConnectionError,"},{"line_number":265,"context_line":"                requests.exceptions.ConnectTimeout) as e:"},{"line_number":266,"context_line":"            LOG.error(_LE(\u0027Failed to proxy to %r: %s\u0027 % (url, e)))"},{"line_number":267,"context_line":"            raise webob.exc.HTTPGatewayTimeout(\u0027Stage host is unavailable\u0027)"},{"line_number":268,"context_line":"        except requests.exceptions.RequestException as e:"},{"line_number":269,"context_line":"            LOG.error(_LE(\u0027Failed to proxy to %r: %s\u0027 % (url, e)))"}],"source_content_type":"text/x-python","patch_set":12,"id":"15d3709b_c8acb35b","line":266,"range":{"start_line":266,"start_character":54,"end_line":266,"end_character":55},"updated":"2021-03-01 18:42:23.000000000","message":"ditto","commit_id":"bc0525f365c6be078506179b4bafa976ce9c1f0a"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"2e16fe18eace96041624e5aee5c261c5168a8d6d","unresolved":true,"context_lines":[{"line_number":266,"context_line":"            LOG.error(_LE(\u0027Failed to proxy to %r: %s\u0027 % (url, e)))"},{"line_number":267,"context_line":"            raise webob.exc.HTTPGatewayTimeout(\u0027Stage host is unavailable\u0027)"},{"line_number":268,"context_line":"        except requests.exceptions.RequestException as e:"},{"line_number":269,"context_line":"            LOG.error(_LE(\u0027Failed to proxy to %r: %s\u0027 % (url, e)))"},{"line_number":270,"context_line":"            raise webob.exc.HTTPBadGateway(\u0027Stage host is unavailable\u0027)"},{"line_number":271,"context_line":"        req_id_hdr \u003d \u0027x-openstack-request-id\u0027"},{"line_number":272,"context_line":"        if req_id_hdr in r.headers:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9073f212_6f080caf","line":269,"range":{"start_line":269,"start_character":54,"end_line":269,"end_character":55},"updated":"2021-03-01 18:42:23.000000000","message":"ditto","commit_id":"bc0525f365c6be078506179b4bafa976ce9c1f0a"}],"glance/async_/flows/_internal_plugins/web_download.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"aa0563fcaa1dd4a886fafec58212e22397b66a10","unresolved":true,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        store.configure()"},{"line_number":99,"context_line":"        return store"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def execute(self):"},{"line_number":102,"context_line":"        \"\"\"Create temp file into store and return path to it"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9a11754b_da1c147f","line":100,"updated":"2021-02-01 18:21:47.000000000","message":"AFAIK, this is a refactor, unrelated to the actual change you\u0027re making here. Please split this out to a different patch to keep the functional changes separate from cleanups.","commit_id":"14b15031a9a1aa85ed14c725ebef6d4d52419fcb"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"99a5c8c424d7446bbb1159a8a9adfdf2b4ba1abb","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        store.configure()"},{"line_number":99,"context_line":"        return store"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def execute(self):"},{"line_number":102,"context_line":"        \"\"\"Create temp file into store and return path to it"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"ec156d4a_ec4a8d6e","line":100,"in_reply_to":"9a11754b_da1c147f","updated":"2021-02-01 20:56:32.000000000","message":"I don\u0027t know how you have used to do it in the Nova world, but leaving code behind that won\u0027t be called in any circumstance is normally quite frowned upon.","commit_id":"14b15031a9a1aa85ed14c725ebef6d4d52419fcb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3b2f34ab608c0d4ef53b8b7674da219108bfe63a","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        store.configure()"},{"line_number":99,"context_line":"        return store"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def execute(self):"},{"line_number":102,"context_line":"        \"\"\"Create temp file into store and return path to it"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"68f1db3a_57739efe","line":100,"in_reply_to":"ec156d4a_ec4a8d6e","updated":"2021-02-01 22:27:37.000000000","message":"This is again the same pattern as seen in image_data.py, and is another good reason why that DRYing activity should stand on its own.\n\nAnd yes, in Nova we definitely see value in separating refactors from functional changes, and setting up feature code to merge in as much isolation as possible for later clarity, backporting, and regression-hunting. We *do* separate things into patches that we land (tested) before we plan to use them, and often put a hold on the ones that shouldn\u0027t merge until the thing that uses them is ready or agreed-upon. It\u0027s more paperwork, but it\u0027s far easier to move trivial, obvious, and not-fully-related things out to separate patches where, once they are agreed upon, need not be re-reviewed while iterating on the meaty patches.","commit_id":"14b15031a9a1aa85ed14c725ebef6d4d52419fcb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"aa0563fcaa1dd4a886fafec58212e22397b66a10","unresolved":true,"context_lines":[{"line_number":129,"context_line":"                        \"expected\": content_length})"},{"line_number":130,"context_line":"                raise exception.ImportTaskError(msg)"},{"line_number":131,"context_line":"        except (KeyError, ValueError):"},{"line_number":132,"context_line":"            pass"},{"line_number":133,"context_line":"        return self._path"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def revert(self, result, **kwargs):"}],"source_content_type":"text/x-python","patch_set":9,"id":"efa234e2_f6ed2da3","line":132,"updated":"2021-02-01 18:21:47.000000000","message":"AFAIK, this is a refactor, unrelated to the actual change you\u0027re making here. Please split this out to a different patch to keep the functional changes separate from cleanups.","commit_id":"14b15031a9a1aa85ed14c725ebef6d4d52419fcb"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"99a5c8c424d7446bbb1159a8a9adfdf2b4ba1abb","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                        \"expected\": content_length})"},{"line_number":130,"context_line":"                raise exception.ImportTaskError(msg)"},{"line_number":131,"context_line":"        except (KeyError, ValueError):"},{"line_number":132,"context_line":"            pass"},{"line_number":133,"context_line":"        return self._path"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def revert(self, result, **kwargs):"}],"source_content_type":"text/x-python","patch_set":9,"id":"8ec9ee8d_0406fad6","line":132,"in_reply_to":"efa234e2_f6ed2da3","updated":"2021-02-01 20:56:32.000000000","message":"Not correct, again. This time the write just happens bit before, on line 67 of PS10 (I\u0027ll highlight that too), instead of this direct store add call and the error handling we had around to secure it.","commit_id":"14b15031a9a1aa85ed14c725ebef6d4d52419fcb"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"99a5c8c424d7446bbb1159a8a9adfdf2b4ba1abb","unresolved":true,"context_lines":[{"line_number":64,"context_line":"        # refer to the comment in the `_ImportToStore.execute` method."},{"line_number":65,"context_line":"        try:"},{"line_number":66,"context_line":"            data \u003d script_utils.get_image_data_iter(self.uri)"},{"line_number":67,"context_line":"            image.set_data(data, staging\u003dTrue)"},{"line_number":68,"context_line":"        except Exception as e:"},{"line_number":69,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":70,"context_line":"                LOG.error(\"Task %(task_id)s failed with exception %(error)s\","}],"source_content_type":"text/x-python","patch_set":10,"id":"908bb60b_cfac5b4b","line":67,"updated":"2021-02-01 20:56:32.000000000","message":"This is where that staging write happens.","commit_id":"8ca338089c24f9ec68523657a7d1c7401ab7d17a"}],"glance/async_/flows/api_image_import.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"aa0563fcaa1dd4a886fafec58212e22397b66a10","unresolved":true,"context_lines":[{"line_number":374,"context_line":"                LOG.warning(_(\"After upload to backend, deletion of staged \""},{"line_number":375,"context_line":"                              \"image data has failed because \""},{"line_number":376,"context_line":"                              \"it cannot be found at %(fn)s\"), {"},{"line_number":377,"context_line":"                    \u0027fn\u0027: file_path})"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"class _ImageLock(task.Task):"}],"source_content_type":"text/x-python","patch_set":9,"id":"918d92cc_da46aaf3","line":377,"updated":"2021-02-01 18:21:47.000000000","message":"AFAIK, this is a refactor, unrelated to the actual change you\u0027re making here. Please split this out to a different patch to keep the functional changes separate from cleanups.","commit_id":"14b15031a9a1aa85ed14c725ebef6d4d52419fcb"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"99a5c8c424d7446bbb1159a8a9adfdf2b4ba1abb","unresolved":false,"context_lines":[{"line_number":374,"context_line":"                LOG.warning(_(\"After upload to backend, deletion of staged \""},{"line_number":375,"context_line":"                              \"image data has failed because \""},{"line_number":376,"context_line":"                              \"it cannot be found at %(fn)s\"), {"},{"line_number":377,"context_line":"                    \u0027fn\u0027: file_path})"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"class _ImageLock(task.Task):"}],"source_content_type":"text/x-python","patch_set":9,"id":"a405d6cf_70f3a42e","line":377,"in_reply_to":"918d92cc_da46aaf3","updated":"2021-02-01 20:56:32.000000000","message":"And wrong again. As we are tracking the staging data, we need to clear it out properly too, not just make the data disappear from the filesystem. Thus we call the locations.pop(i) and let the locations/store code to handle the delete. Btw this will also clear up the metadata \u0027staging_host\u0027 you were so worried we will be storing forever.","commit_id":"14b15031a9a1aa85ed14c725ebef6d4d52419fcb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3b2f34ab608c0d4ef53b8b7674da219108bfe63a","unresolved":false,"context_lines":[{"line_number":374,"context_line":"                LOG.warning(_(\"After upload to backend, deletion of staged \""},{"line_number":375,"context_line":"                              \"image data has failed because \""},{"line_number":376,"context_line":"                              \"it cannot be found at %(fn)s\"), {"},{"line_number":377,"context_line":"                    \u0027fn\u0027: file_path})"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"class _ImageLock(task.Task):"}],"source_content_type":"text/x-python","patch_set":9,"id":"b920289a_6610e5cc","line":377,"in_reply_to":"a405d6cf_70f3a42e","updated":"2021-02-01 22:27:37.000000000","message":"Again, this is the nearly the same pattern as first changed in image_data.py. AFAICT, if we arranged that store in front of the proxy patch, the proxy would only need to add new lines L352-3 to the logic. Instead, it looks like this DRYing is related to the proxying, which I don\u0027t think it really is.","commit_id":"14b15031a9a1aa85ed14c725ebef6d4d52419fcb"}],"glance/common/config.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"babd3c7006ae590883103cd362cc4be3a35342ff","unresolved":true,"context_lines":[{"line_number":565,"context_line":""},{"line_number":566,"context_line":"    Related options:"},{"line_number":567,"context_line":"        * [DEFAULT]/node_staging_uri\"\"\")),"},{"line_number":568,"context_line":"    cfg.StrOpt(\u0027direct_url\u0027,"},{"line_number":569,"context_line":"               default\u003dNone,"},{"line_number":570,"context_line":"               help\u003d_(\"\"\""},{"line_number":571,"context_line":"Direct URL to this worker."}],"source_content_type":"text/x-python","patch_set":2,"id":"e317fbea_356a18d1","line":568,"range":{"start_line":568,"start_character":16,"end_line":568,"end_character":26},"updated":"2021-01-11 17:13:59.000000000","message":"Glance already uses \u0027direct_url\u0027 as the name for an image property whose display is controlled by the show_image_direct_url config option, so I\u0027d prefer to use a different name for this.  Maybe something along the lines of worker_self_reference_url (I\u0027m sure you can come up with something better).","commit_id":"0f49b81f908f60355e6ac9de6e55e200673488b8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c380162712f87246c9f837ca50cb9393d3abb393","unresolved":true,"context_lines":[{"line_number":565,"context_line":""},{"line_number":566,"context_line":"    Related options:"},{"line_number":567,"context_line":"        * [DEFAULT]/node_staging_uri\"\"\")),"},{"line_number":568,"context_line":"    cfg.StrOpt(\u0027direct_url\u0027,"},{"line_number":569,"context_line":"               default\u003dNone,"},{"line_number":570,"context_line":"               help\u003d_(\"\"\""},{"line_number":571,"context_line":"Direct URL to this worker."}],"source_content_type":"text/x-python","patch_set":2,"id":"7ee067ae_eef3a69c","line":568,"range":{"start_line":568,"start_character":16,"end_line":568,"end_character":26},"in_reply_to":"e317fbea_356a18d1","updated":"2021-01-11 18:19:19.000000000","message":"Sure, good to avoid the confusion. This is the fewest characters I could type for a PoC :)","commit_id":"0f49b81f908f60355e6ac9de6e55e200673488b8"}],"glance/context.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"8ca278d63673bcb029e07ee426204b9a94c06f22","unresolved":true,"context_lines":[{"line_number":22,"context_line":"from glance.api import policy"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class _ContextAuthPlugin(plugin.BaseAuthPlugin):"},{"line_number":26,"context_line":"    \"\"\"A keystoneauth auth plugin that uses the values from the Context.\"\"\""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def __init__(self, auth_token):"}],"source_content_type":"text/x-python","patch_set":1,"id":"506eea79_096f2810","line":25,"range":{"start_line":25,"start_character":6,"end_line":25,"end_character":24},"updated":"2021-01-08 21:30:19.000000000","message":"You might be able to use https://docs.openstack.org/keystoneauth/latest/authentication-plugins.html#simple-plugins instead. It looks like it does effectively the same thing as _ContextAuthPlugin, with the advantage of not sandwiching a validation request in between two service calls.","commit_id":"24874aacbaf2ea759640ac8ad090998128f909c0"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"8ca278d63673bcb029e07ee426204b9a94c06f22","unresolved":true,"context_lines":[{"line_number":41,"context_line":"    \"\"\""},{"line_number":42,"context_line":"    # FIXME(danms): This should work, apparently, but does not. When it is"},{"line_number":43,"context_line":"    # fixed replace the above shim with this:"},{"line_number":44,"context_line":"    # auth \u003d keystoneauth1.identity.v3.TokenMethod(token\u003dcontext.auth_token)"},{"line_number":45,"context_line":"    auth \u003d _ContextAuthPlugin(context.auth_token)"},{"line_number":46,"context_line":"    return session.Session(auth\u003dauth)"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1f520880_f4f0a100","line":44,"updated":"2021-01-08 21:30:19.000000000","message":"You could replace this with\n\n  from keystoneauth1 import session\n  from keystoneauth1 import token_endpoint\n  from keystoneauth1.access import service_catalog\n\n  ...\n\n  def build_ksa_client(req):\n      service_catalog_string \u003d req.headers.get(\u0027HTTP_X_SERVICE_CATALOG\u0027)\n      sc \u003d service_catalog.ServiceCatalogV2(json.loads(service_catalog_string))\n      identity_endpoint \u003d sc.url_for(service_type\u003d\u0027identity\u0027)\n      auth \u003d token_endpoint.Token(identity_endpoint, req.context.auth_token)\n      return session.Session(auth\u003dauth)","commit_id":"24874aacbaf2ea759640ac8ad090998128f909c0"}],"glance/location.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"aa0563fcaa1dd4a886fafec58212e22397b66a10","unresolved":true,"context_lines":[{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        self.value.insert(i, location)"},{"line_number":235,"context_line":"        if not location[\u0027metadata\u0027].get(\u0027staging_host\u0027):"},{"line_number":236,"context_line":"            # NOTE(jokke): set the image size only if this is not staging"},{"line_number":237,"context_line":"            _set_image_size(self.image_proxy.context,"},{"line_number":238,"context_line":"                            self.image_proxy,"},{"line_number":239,"context_line":"                            [location])"}],"source_content_type":"text/x-python","patch_set":10,"id":"cfb7ef55_a03211e0","line":236,"updated":"2021-02-01 18:21:47.000000000","message":"Why?","commit_id":"8ca338089c24f9ec68523657a7d1c7401ab7d17a"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"99a5c8c424d7446bbb1159a8a9adfdf2b4ba1abb","unresolved":false,"context_lines":[{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        self.value.insert(i, location)"},{"line_number":235,"context_line":"        if not location[\u0027metadata\u0027].get(\u0027staging_host\u0027):"},{"line_number":236,"context_line":"            # NOTE(jokke): set the image size only if this is not staging"},{"line_number":237,"context_line":"            _set_image_size(self.image_proxy.context,"},{"line_number":238,"context_line":"                            self.image_proxy,"},{"line_number":239,"context_line":"                            [location])"}],"source_content_type":"text/x-python","patch_set":10,"id":"d0cfd3f1_0bd67593","line":236,"in_reply_to":"cfb7ef55_a03211e0","updated":"2021-02-01 20:56:32.000000000","message":"Because we do not know yet what the actual image size will be.","commit_id":"8ca338089c24f9ec68523657a7d1c7401ab7d17a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3b2f34ab608c0d4ef53b8b7674da219108bfe63a","unresolved":false,"context_lines":[{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        self.value.insert(i, location)"},{"line_number":235,"context_line":"        if not location[\u0027metadata\u0027].get(\u0027staging_host\u0027):"},{"line_number":236,"context_line":"            # NOTE(jokke): set the image size only if this is not staging"},{"line_number":237,"context_line":"            _set_image_size(self.image_proxy.context,"},{"line_number":238,"context_line":"                            self.image_proxy,"},{"line_number":239,"context_line":"                            [location])"}],"source_content_type":"text/x-python","patch_set":10,"id":"0d151bb8_39fbfdc5","line":236,"in_reply_to":"d0cfd3f1_0bd67593","updated":"2021-02-01 22:27:37.000000000","message":"Ack, so a tempest test that can see the locations should be able to validate that the image gets a new location after stage and that the image size is zero, right?","commit_id":"8ca338089c24f9ec68523657a7d1c7401ab7d17a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"aa0563fcaa1dd4a886fafec58212e22397b66a10","unresolved":true,"context_lines":[{"line_number":551,"context_line":""},{"line_number":552,"context_line":"            if loc_meta is None:"},{"line_number":553,"context_line":"                loc_meta \u003d {}"},{"line_number":554,"context_line":"            loc_meta[\u0027staging_host\u0027] \u003d CONF.direct_access_url"},{"line_number":555,"context_line":""},{"line_number":556,"context_line":"        self.image.locations.append({\u0027url\u0027: location, \u0027metadata\u0027: loc_meta,"},{"line_number":557,"context_line":"                                     \u0027status\u0027: \u0027active\u0027})"}],"source_content_type":"text/x-python","patch_set":10,"id":"d6ae060c_87394ec6","line":554,"updated":"2021-02-01 18:21:47.000000000","message":"Surely this can and should be broken up into at least two pieces, one for a \"normal\" store and another for the \"special case staging store\".\n\nThis seems to be a giant special case to avoid setting certain image properties which normally get set when we add the first location, is that right?\n\nI would also expect that everything except the last line here is part of the refactor you\u0027re doing in other places where you remove the \"if CONF.enabled_backends\" branch. Thus, I would definitely expect those, along with this, to be in a separate patch that we can validate and test independently to make sure it doesn\u0027t regress.","commit_id":"8ca338089c24f9ec68523657a7d1c7401ab7d17a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3b2f34ab608c0d4ef53b8b7674da219108bfe63a","unresolved":false,"context_lines":[{"line_number":551,"context_line":""},{"line_number":552,"context_line":"            if loc_meta is None:"},{"line_number":553,"context_line":"                loc_meta \u003d {}"},{"line_number":554,"context_line":"            loc_meta[\u0027staging_host\u0027] \u003d CONF.direct_access_url"},{"line_number":555,"context_line":""},{"line_number":556,"context_line":"        self.image.locations.append({\u0027url\u0027: location, \u0027metadata\u0027: loc_meta,"},{"line_number":557,"context_line":"                                     \u0027status\u0027: \u0027active\u0027})"}],"source_content_type":"text/x-python","patch_set":10,"id":"f470ff35_01202596","line":554,"in_reply_to":"637d3486_cece7f47","updated":"2021-02-01 22:27:37.000000000","message":"\u003e As you probably can see, the non-staging code has not changed. And no, not looking to split it across the whole codebase into helper functions. This will be much easier to clean up when it\u0027s all in the same place.\n\nI can, but it\u0027s difficult to eye-parse this because you changed every single line while also adding new functionality. Separating this out into its own piece means it can be validated easily to have not changed (besides becoming an inner function that is called on either side of the \"if not staging\" conditional.\n\nIt also seems very odd to me to have something this deep in the bowels need to know that the staging store is special. To me, that indicates a layering violation, akin to your editor needing to know what filesystem is being used to properly save your text files. It\u0027s this sort of thing that I think deserves some discussion and review, on its own and not mired in the other discussion of how this distributed import should be done. If this was a separate patch, then we could have that discussion in a patch about just that topic.\n\n\u003e Mainly two fold, making sure we do not set properties like size or multihash yet as we don\u0027t know what their values should be and that we inject the needed config values to the conf when initializing the staging store on non-multi-store environment to make it possible. This was done in multiple (staging api + all the rest of the import jobs) places before but using the locations to track the staging data allows us to clean that up, like explained in the spec.\n\nWell, it\u0027s clear that my opinion on this matter differs, so feel free to ignore. However, to my eyes, it\u0027s much more difficult to look a giant if statement (which only just barely fits on my 5K screen) and convince myself that it\u0027s correct. It\u0027s far easier for us to test and validate two smaller functions in isolation, along with the logic for why we call one and not the other.","commit_id":"8ca338089c24f9ec68523657a7d1c7401ab7d17a"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"99a5c8c424d7446bbb1159a8a9adfdf2b4ba1abb","unresolved":false,"context_lines":[{"line_number":551,"context_line":""},{"line_number":552,"context_line":"            if loc_meta is None:"},{"line_number":553,"context_line":"                loc_meta \u003d {}"},{"line_number":554,"context_line":"            loc_meta[\u0027staging_host\u0027] \u003d CONF.direct_access_url"},{"line_number":555,"context_line":""},{"line_number":556,"context_line":"        self.image.locations.append({\u0027url\u0027: location, \u0027metadata\u0027: loc_meta,"},{"line_number":557,"context_line":"                                     \u0027status\u0027: \u0027active\u0027})"}],"source_content_type":"text/x-python","patch_set":10,"id":"637d3486_cece7f47","line":554,"in_reply_to":"d6ae060c_87394ec6","updated":"2021-02-01 20:56:32.000000000","message":"This will indeed clean up even more once we get to remove the non-multi-store config model. Until the cycle we get to do that, the later part _is_ the staging code.\n\nAs you probably can see, the non-staging code has not changed. And no, not looking to split it across the whole codebase into helper functions. This will be much easier to clean up when it\u0027s all in the same place.\n\nMainly two fold, making sure we do not set properties like size or multihash yet as we don\u0027t know what their values should be and that we inject the needed config values to the conf when initializing the staging store on non-multi-store environment to make it possible. This was done in multiple (staging api + all the rest of the import jobs) places before but using the locations to track the staging data allows us to clean that up, like explained in the spec.","commit_id":"8ca338089c24f9ec68523657a7d1c7401ab7d17a"}]}