)]}'
{"glance/api/v2/image_data.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"329979700c00dc3c9c970b2fbe0e96ced1da57a7","unresolved":true,"context_lines":[{"line_number":35,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"CONF \u003d cfg.CONF"},{"line_number":38,"context_line":"CONF.import_opt(\u0027public_endpoint\u0027, \u0027glance.api.versions\u0027)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"class ImageDataController(object):"}],"source_content_type":"text/x-python","patch_set":2,"id":"098d60dc_c7e23e55","line":38,"updated":"2021-03-01 14:41:59.000000000","message":"Stale?","commit_id":"ec9de83e4665a3dbdd75b4f89d642451fe431688"}],"glance/api/v2/images.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"367d8ba5a82cbd742c36fa7198d05581afebdd98","unresolved":true,"context_lines":[{"line_number":256,"context_line":"                stage_host \u003d loc[\u0027staging_host\u0027]"},{"line_number":257,"context_line":"                break"},{"line_number":258,"context_line":"        LOG.info(_LI(\u0027Proxying %s request to host %s \u0027"},{"line_number":259,"context_line":"                     \u0027which has image staged\u0027) % (req.method, stage_host))"},{"line_number":260,"context_line":"        client \u003d glance_context.get_ksa_client(req.context)"},{"line_number":261,"context_line":"        url \u003d \u0027%s%s\u0027 % (stage_host, req.path)"},{"line_number":262,"context_line":"        req_id_hdr \u003d \u0027x-openstack-request-id\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6a7659_223049c5","line":259,"range":{"start_line":259,"start_character":46,"end_line":259,"end_character":73},"updated":"2021-03-01 13:21:32.000000000","message":"Nit: should be \", req.method, stage_host\".","commit_id":"ec9de83e4665a3dbdd75b4f89d642451fe431688"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"367d8ba5a82cbd742c36fa7198d05581afebdd98","unresolved":true,"context_lines":[{"line_number":265,"context_line":"            r \u003d request_method(url, json\u003dbody, timeout\u003d60)"},{"line_number":266,"context_line":"        except (requests.exceptions.ConnectionError,"},{"line_number":267,"context_line":"                requests.exceptions.ConnectTimeout) as e:"},{"line_number":268,"context_line":"            LOG.error(_LE(\u0027Failed to proxy to %r: %s\u0027 % (url, e)))"},{"line_number":269,"context_line":"            raise webob.exc.HTTPGatewayTimeout(\u0027Stage host is unavailable\u0027)"},{"line_number":270,"context_line":"        except requests.exceptions.RequestException as e:"},{"line_number":271,"context_line":"            LOG.error(_LE(\u0027Failed to proxy to %r: %s\u0027 % (url, e)))"}],"source_content_type":"text/x-python","patch_set":2,"id":"b8dae5e1_61d0b5e8","line":268,"range":{"start_line":268,"start_character":54,"end_line":268,"end_character":57},"updated":"2021-03-01 13:21:32.000000000","message":"Throughout","commit_id":"ec9de83e4665a3dbdd75b4f89d642451fe431688"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"329979700c00dc3c9c970b2fbe0e96ced1da57a7","unresolved":true,"context_lines":[{"line_number":298,"context_line":"        :returns: bool indicating proxyable status"},{"line_number":299,"context_line":"        \"\"\""},{"line_number":300,"context_line":"        for loc in image.locations:"},{"line_number":301,"context_line":"            staging_host \u003d loc.get(\u0027staging_host\u0027)"},{"line_number":302,"context_line":"            if staging_host and staging_host !\u003d \u0027not_set\u0027:"},{"line_number":303,"context_line":"                return (staging_host !\u003d self.self_url)"},{"line_number":304,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"312d4c1c_993dd3b8","line":301,"range":{"start_line":301,"start_character":31,"end_line":301,"end_character":34},"updated":"2021-03-01 14:41:59.000000000","message":"Here and elsewhere...why are you using .get() for this? You\u0027ve added the column, it should always be there. Why introduce the ambiguity? Maybe just as a shortcut to avoid breaking more tests? Either way, these should just be direct, IMHO.","commit_id":"ec9de83e4665a3dbdd75b4f89d642451fe431688"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"329979700c00dc3c9c970b2fbe0e96ced1da57a7","unresolved":true,"context_lines":[{"line_number":299,"context_line":"        \"\"\""},{"line_number":300,"context_line":"        for loc in image.locations:"},{"line_number":301,"context_line":"            staging_host \u003d loc.get(\u0027staging_host\u0027)"},{"line_number":302,"context_line":"            if staging_host and staging_host !\u003d \u0027not_set\u0027:"},{"line_number":303,"context_line":"                return (staging_host !\u003d self.self_url)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @utils.mutating"}],"source_content_type":"text/x-python","patch_set":2,"id":"af04f98a_c1e4d77f","line":302,"range":{"start_line":302,"start_character":15,"end_line":302,"end_character":27},"updated":"2021-03-01 14:41:59.000000000","message":"Doesn\u0027t this go against your argument for adding a staging boolean to the database in the bottom patch of this series?","commit_id":"ec9de83e4665a3dbdd75b4f89d642451fe431688"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"329979700c00dc3c9c970b2fbe0e96ced1da57a7","unresolved":true,"context_lines":[{"line_number":299,"context_line":"        \"\"\""},{"line_number":300,"context_line":"        for loc in image.locations:"},{"line_number":301,"context_line":"            staging_host \u003d loc.get(\u0027staging_host\u0027)"},{"line_number":302,"context_line":"            if staging_host and staging_host !\u003d \u0027not_set\u0027:"},{"line_number":303,"context_line":"                return (staging_host !\u003d self.self_url)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @utils.mutating"}],"source_content_type":"text/x-python","patch_set":2,"id":"2fc45a32_eb008454","line":302,"range":{"start_line":302,"start_character":49,"end_line":302,"end_character":56},"updated":"2021-03-01 14:41:59.000000000","message":"This does not seem like a reasonably unique sentinel to me.","commit_id":"ec9de83e4665a3dbdd75b4f89d642451fe431688"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"81b38aa0cb262c8040a564a98717848c48935972","unresolved":true,"context_lines":[{"line_number":299,"context_line":"        \"\"\""},{"line_number":300,"context_line":"        for loc in image.locations:"},{"line_number":301,"context_line":"            staging_host \u003d loc.get(\u0027staging_host\u0027)"},{"line_number":302,"context_line":"            if staging_host and staging_host !\u003d \u0027not_set\u0027:"},{"line_number":303,"context_line":"                return (staging_host !\u003d self.self_url)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @utils.mutating"}],"source_content_type":"text/x-python","patch_set":2,"id":"d0b63667_392db687","line":302,"range":{"start_line":302,"start_character":15,"end_line":302,"end_character":27},"in_reply_to":"af04f98a_c1e4d77f","updated":"2021-03-01 16:17:11.000000000","message":"yes for both, and deserves cleanup. Good candidate for follow up, right ;)","commit_id":"ec9de83e4665a3dbdd75b4f89d642451fe431688"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"329979700c00dc3c9c970b2fbe0e96ced1da57a7","unresolved":true,"context_lines":[{"line_number":776,"context_line":"                return"},{"line_number":777,"context_line":"            elif image.locations:"},{"line_number":778,"context_line":"                for loc in image.locations:"},{"line_number":779,"context_line":"                    if loc.get(\u0027staging_host\u0027):"},{"line_number":780,"context_line":"                        msg \u003d _(\"Got error from staging host, unable to \""},{"line_number":781,"context_line":"                                \"delete the staged data location.\")"},{"line_number":782,"context_line":"                        raise webob.exc.HTTPServiceUnavailable(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f5af0855_3168c355","line":779,"range":{"start_line":779,"start_character":23,"end_line":779,"end_character":47},"updated":"2021-03-01 14:41:59.000000000","message":"Again, using this as a boolean :)","commit_id":"ec9de83e4665a3dbdd75b4f89d642451fe431688"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"81b38aa0cb262c8040a564a98717848c48935972","unresolved":true,"context_lines":[{"line_number":776,"context_line":"                return"},{"line_number":777,"context_line":"            elif image.locations:"},{"line_number":778,"context_line":"                for loc in image.locations:"},{"line_number":779,"context_line":"                    if loc.get(\u0027staging_host\u0027):"},{"line_number":780,"context_line":"                        msg \u003d _(\"Got error from staging host, unable to \""},{"line_number":781,"context_line":"                                \"delete the staged data location.\")"},{"line_number":782,"context_line":"                        raise webob.exc.HTTPServiceUnavailable(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":2,"id":"26880d7a_12d00bdb","line":779,"range":{"start_line":779,"start_character":23,"end_line":779,"end_character":47},"in_reply_to":"f5af0855_3168c355","updated":"2021-03-01 16:17:11.000000000","message":"ditto","commit_id":"ec9de83e4665a3dbdd75b4f89d642451fe431688"}],"glance/location.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"329979700c00dc3c9c970b2fbe0e96ced1da57a7","unresolved":true,"context_lines":[{"line_number":544,"context_line":"            staging_host \u003d CONF.direct_access_url or CONF.public_endpoint"},{"line_number":545,"context_line":"            # NOTE(jokke) If neither config option is defined we put \"not_set\""},{"line_number":546,"context_line":"            # as staging_host, this simplifies the logic consuming the value"},{"line_number":547,"context_line":"            # significantly."},{"line_number":548,"context_line":"            staging_host \u003d staging_host or \"not_set\""},{"line_number":549,"context_line":""},{"line_number":550,"context_line":"        self.image.locations.append({\u0027url\u0027: location, \u0027metadata\u0027: loc_meta,"}],"source_content_type":"text/x-python","patch_set":2,"id":"361dfa46_1a7bb14d","line":547,"updated":"2021-03-01 14:41:59.000000000","message":"How does it make it simpler? In my version, we compare staging_host to our copy of either of those, and if different, we proxy. With this, you now have this magic string, which someone *could* set in the config and then end up very confused. Instead of getting an \"unable to connect to not_set\" and realizing their error, they\u0027d see nothing.","commit_id":"ec9de83e4665a3dbdd75b4f89d642451fe431688"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"329979700c00dc3c9c970b2fbe0e96ced1da57a7","unresolved":true,"context_lines":[{"line_number":545,"context_line":"            # NOTE(jokke) If neither config option is defined we put \"not_set\""},{"line_number":546,"context_line":"            # as staging_host, this simplifies the logic consuming the value"},{"line_number":547,"context_line":"            # significantly."},{"line_number":548,"context_line":"            staging_host \u003d staging_host or \"not_set\""},{"line_number":549,"context_line":""},{"line_number":550,"context_line":"        self.image.locations.append({\u0027url\u0027: location, \u0027metadata\u0027: loc_meta,"},{"line_number":551,"context_line":"                                     \u0027status\u0027: \u0027active\u0027, \u0027staging\u0027: staging,"}],"source_content_type":"text/x-python","patch_set":2,"id":"393551f3_3f6f15ae","line":548,"updated":"2021-03-01 14:41:59.000000000","message":"As noted elsewhere, I think setting this deep in the location code, according to this special-case staging flag, is a layering violation. This URL is the business of the API, and it has to do all the comparisons, so setting it should *also* be done there.","commit_id":"ec9de83e4665a3dbdd75b4f89d642451fe431688"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"81b38aa0cb262c8040a564a98717848c48935972","unresolved":true,"context_lines":[{"line_number":545,"context_line":"            # NOTE(jokke) If neither config option is defined we put \"not_set\""},{"line_number":546,"context_line":"            # as staging_host, this simplifies the logic consuming the value"},{"line_number":547,"context_line":"            # significantly."},{"line_number":548,"context_line":"            staging_host \u003d staging_host or \"not_set\""},{"line_number":549,"context_line":""},{"line_number":550,"context_line":"        self.image.locations.append({\u0027url\u0027: location, \u0027metadata\u0027: loc_meta,"},{"line_number":551,"context_line":"                                     \u0027status\u0027: \u0027active\u0027, \u0027staging\u0027: staging,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7793f377_41a5992f","line":548,"in_reply_to":"393551f3_3f6f15ae","updated":"2021-03-01 16:17:11.000000000","message":"yup like said this likely deserves following cleanup.","commit_id":"ec9de83e4665a3dbdd75b4f89d642451fe431688"}]}