)]}'
{"glance/api/authorization.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"162fed402c08cb53fa5b18c7580e1438412c8104","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    \"\"\"Update store information in location metadata\"\"\""},{"line_number":34,"context_line":"    @functools.wraps(func)"},{"line_number":35,"context_line":"    def wrapped(context, image, **kwargs):"},{"line_number":36,"context_line":"        image_repo \u003d kwargs.get(\u0027image_repo\u0027)"},{"line_number":37,"context_line":"        if CONF.enabled_backends:"},{"line_number":38,"context_line":"            for loc in image.locations:"},{"line_number":39,"context_line":"                store_id \u003d store_utils.get_store_id_from_uri(loc[\u0027url\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_7336c053","line":36,"updated":"2019-07-19 13:34:00.000000000","message":"It\u0027s possible that this could be None (see line 73), which will cause an AttributeError at line 55.  Maybe change line 37 to \"if image_repo and CONF.enabled_backends:\" ?","commit_id":"32598c624e760ef66c130f668a9e729207efb7ac"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9a0f16c65db5dd84a008a5314c45adbc16fb90c8","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    \"\"\"Update store information in location metadata\"\"\""},{"line_number":34,"context_line":"    @functools.wraps(func)"},{"line_number":35,"context_line":"    def wrapped(context, image, **kwargs):"},{"line_number":36,"context_line":"        image_repo \u003d kwargs.get(\u0027image_repo\u0027)"},{"line_number":37,"context_line":"        if CONF.enabled_backends:"},{"line_number":38,"context_line":"            for loc in image.locations:"},{"line_number":39,"context_line":"                store_id \u003d store_utils.get_store_id_from_uri(loc[\u0027url\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_4f18a583","line":36,"in_reply_to":"7faddb67_7336c053","updated":"2019-07-22 05:13:04.000000000","message":"Done","commit_id":"32598c624e760ef66c130f668a9e729207efb7ac"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d49cf2872d1c206f43c74529c3fd764d126667f2","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                            continue"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"                    loc[\u0027metadata\u0027][\u0027backend\u0027] \u003d store_id"},{"line_number":55,"context_line":"                    image_repo.save(image)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        return func(context, image, **kwargs)"},{"line_number":58,"context_line":"    return wrapped"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_5f2022f8","line":55,"updated":"2019-07-24 16:59:29.000000000","message":"I may be thinking about this the wrong way, so I\u0027ll explain my problem and you can tell me what I\u0027m missing.\n\nSuppose an operator has enabled_backends configured like in our unit tests:\nenabled_backends \u003d fast:file, cheap:file, read_only:http\n\nA new image gets added, so it will have a location with the correct metadata\n{\u0027id\u0027: \u00275e714993-559c-4839-ab17-e3711c851bd9\u0027, \u0027url\u0027: \u0027file:///tmp/tmpvljwt2p1/a85abd86-55b3-4d5b-b0b4-5d0a6e6042fc\u0027, \u0027metadata\u0027: {\u0027backend\u0027: \u0027fast\u0027}, \u0027status\u0027: \u0027active\u0027}\n\nWe do an image-list call.  The lazy update grabs the location and calls store_utils.get_store_id_from_uri(loc[\u0027url\u0027]).  It gets \u0027cheap\u0027 because that comes first in the map.  This is different from the store_id in the location metadata, so it logs a warning and updates the metadata -- which is now incorrect.\n\nI need you to explain why that can\u0027t happen.","commit_id":"0244da4fd1afe3fb2d4fae966a622a407a06fc1d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a72598dc9a22388f945868e3fa9a973c6e62af28","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                            continue"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"                    loc[\u0027metadata\u0027][\u0027backend\u0027] \u003d store_id"},{"line_number":55,"context_line":"                    image_repo.save(image)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        return func(context, image, **kwargs)"},{"line_number":58,"context_line":"    return wrapped"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_02d56db9","line":55,"in_reply_to":"7faddb67_5f2022f8","updated":"2019-07-24 18:05:03.000000000","message":"Hi Brian,\nThis will not happen because, fast and cheap will have different file paths.\nFor example fast will have path as file:///tmp/tmp_fast and cheap will have path as file:///tmp/tmp_cheap.\n\nIf you look at the method from store_utils, then it is checking the location url is starts with store.url_prefix in this case it will return True only for fast and not cheap.\n\nLet me know whether this satisfies your doubt.","commit_id":"0244da4fd1afe3fb2d4fae966a622a407a06fc1d"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"537b2cab5a5ffe59934d297f59a046f8f87b4399","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                    if \u0027backend\u0027 in loc[\u0027metadata\u0027]:"},{"line_number":42,"context_line":"                        old_store \u003d loc[\u0027metadata\u0027][\u0027backend\u0027]"},{"line_number":43,"context_line":"                        if old_store !\u003d store_id:"},{"line_number":44,"context_line":"                            LOG.warning(_(\"Store \u0027%(old)s\u0027 has changed to \""},{"line_number":45,"context_line":"                                          \"\u0027%(new)s\u0027 by operator, updating \""},{"line_number":46,"context_line":"                                          \"the same in the location of image \""},{"line_number":47,"context_line":"                                          \"\u0027%(id)s\u0027\") % {"},{"line_number":48,"context_line":"                                \u0027old\u0027: old_store,"},{"line_number":49,"context_line":"                                \u0027new\u0027: store_id,"},{"line_number":50,"context_line":"                                \u0027id\u0027: image.image_id})"},{"line_number":51,"context_line":"                        else:"},{"line_number":52,"context_line":"                            continue"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_22f97e7f","line":50,"range":{"start_line":44,"start_character":0,"end_line":50,"end_character":54},"updated":"2019-07-25 11:38:44.000000000","message":"I have same problem here as in the glance_store patch.\n\nWhy warning? Isn\u0027t this something we expect to happen and is not something operator should take action on? This will potentially dump thousands of warnings into the logs on something that is normal operation and expected making it even worse than it happening when updating locations in glance_store.\n\nIMHO this should be debut or at max info, unless we have strong case why every single one of these should be highlighted to the operator.","commit_id":"f2785ab8b15a7f960aafe538ea50c4bdd91ac7b9"}],"glance/common/store_utils.py":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"537b2cab5a5ffe59934d297f59a046f8f87b4399","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        return store"},{"line_number":166,"context_line":"    else:"},{"line_number":167,"context_line":"        reason \u003d (_(\"Invalid location uri %s\") % uri)"},{"line_number":168,"context_line":"        LOG.warning(reason)"},{"line_number":169,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_a22a4ee4","line":168,"range":{"start_line":168,"start_character":8,"end_line":168,"end_character":27},"updated":"2019-07-25 11:38:44.000000000","message":"Here the warning is very much appropriate.","commit_id":"f2785ab8b15a7f960aafe538ea50c4bdd91ac7b9"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"7186b1f590fd57bbdad350071a26ec79e873e6d2","unresolved":false,"context_lines":[{"line_number":153,"context_line":"    scheme \u003d urlparse.urlparse(uri).scheme"},{"line_number":154,"context_line":"    location_map \u003d store_api.location.SCHEME_TO_CLS_BACKEND_MAP"},{"line_number":155,"context_line":"    url_matched \u003d False"},{"line_number":156,"context_line":"    if scheme not in location_map:"},{"line_number":157,"context_line":"        return"},{"line_number":158,"context_line":"    for store in location_map[scheme]:"},{"line_number":159,"context_line":"        store_instance \u003d location_map[scheme][store][\u0027store\u0027]"}],"source_content_type":"text/x-python","patch_set":11,"id":"7faddb67_2a9b5773","line":156,"updated":"2019-07-30 13:28:16.000000000","message":"just curious why we aren\u0027t logging here -- this seems very similar to the situation at line 166.  Here a location URL is using a scheme that\u0027s not in enabled_backends, whereas at line 166 it\u0027s a registered scheme, but it doesn\u0027t correspond to any of the stores listed in enabled_backends.","commit_id":"5cab654f6e5c110e8917e819e53d8f33e5d6fc34"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7ed5cb944230aa1f74d24c5556b1887781887cfc","unresolved":false,"context_lines":[{"line_number":153,"context_line":"    scheme \u003d urlparse.urlparse(uri).scheme"},{"line_number":154,"context_line":"    location_map \u003d store_api.location.SCHEME_TO_CLS_BACKEND_MAP"},{"line_number":155,"context_line":"    url_matched \u003d False"},{"line_number":156,"context_line":"    if scheme not in location_map:"},{"line_number":157,"context_line":"        return"},{"line_number":158,"context_line":"    for store in location_map[scheme]:"},{"line_number":159,"context_line":"        store_instance \u003d location_map[scheme][store][\u0027store\u0027]"}],"source_content_type":"text/x-python","patch_set":11,"id":"7faddb67_002f573f","line":156,"in_reply_to":"7faddb67_2a9b5773","updated":"2019-07-30 17:24:04.000000000","message":"Done","commit_id":"5cab654f6e5c110e8917e819e53d8f33e5d6fc34"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"aeb6b70cc4afc3cfc8d7271527bda3803bcb62ab","unresolved":false,"context_lines":[{"line_number":156,"context_line":"    if scheme not in location_map:"},{"line_number":157,"context_line":"        reason \u003d (_(\"Unknown scheme \u0027%(scheme)s\u0027 found in uri \u0027%(uri)s\u0027\"), {"},{"line_number":158,"context_line":"            \u0027scheme\u0027: scheme, \u0027uri\u0027: uri})"},{"line_number":159,"context_line":"        LOG.warning(reason)"},{"line_number":160,"context_line":"        return"},{"line_number":161,"context_line":"    for store in location_map[scheme]:"},{"line_number":162,"context_line":"        store_instance \u003d location_map[scheme][store][\u0027store\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_4ef9d9c5","line":159,"updated":"2019-08-01 16:39:57.000000000","message":"Check with Sean (he knows this better than I do), but instead of passing in a string and a dict, you\u0027re passing in a single argument (a tuple of string and dict).  I don\u0027t think it will break anything, but the log message may not be formatted correctly.  I think you may need to unpack the tuple by passing in *reason, or separate the reason from the data dict and pass them in as separate arguments.  Or maybe oslo_logging is smart enough to handle this properly.","commit_id":"ffb277d97751d334ae4ae5665854495239200861"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"d0015a677ec039622e80429da21851529c5a4ef8","unresolved":false,"context_lines":[{"line_number":156,"context_line":"    if scheme not in location_map:"},{"line_number":157,"context_line":"        reason \u003d (_(\"Unknown scheme \u0027%(scheme)s\u0027 found in uri \u0027%(uri)s\u0027\"), {"},{"line_number":158,"context_line":"            \u0027scheme\u0027: scheme, \u0027uri\u0027: uri})"},{"line_number":159,"context_line":"        LOG.warning(reason)"},{"line_number":160,"context_line":"        return"},{"line_number":161,"context_line":"    for store in location_map[scheme]:"},{"line_number":162,"context_line":"        store_instance \u003d location_map[scheme][store][\u0027store\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_d1becc63","line":159,"in_reply_to":"7faddb67_2e425dac","updated":"2019-08-01 17:57:38.000000000","message":"Done","commit_id":"ffb277d97751d334ae4ae5665854495239200861"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"85319c8e45a1bc8a06a11cd9e37720dac69d5e7a","unresolved":false,"context_lines":[{"line_number":156,"context_line":"    if scheme not in location_map:"},{"line_number":157,"context_line":"        reason \u003d (_(\"Unknown scheme \u0027%(scheme)s\u0027 found in uri \u0027%(uri)s\u0027\"), {"},{"line_number":158,"context_line":"            \u0027scheme\u0027: scheme, \u0027uri\u0027: uri})"},{"line_number":159,"context_line":"        LOG.warning(reason)"},{"line_number":160,"context_line":"        return"},{"line_number":161,"context_line":"    for store in location_map[scheme]:"},{"line_number":162,"context_line":"        store_instance \u003d location_map[scheme][store][\u0027store\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_2e425dac","line":159,"in_reply_to":"7faddb67_4ef9d9c5","updated":"2019-08-01 16:51:20.000000000","message":"You are correct Brian, this will not work as expected. Get rid of the reason variable and just move the string and formatting args into the LOG.warning() call as-is and it will be good.","commit_id":"ffb277d97751d334ae4ae5665854495239200861"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"aeb6b70cc4afc3cfc8d7271527bda3803bcb62ab","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        return store"},{"line_number":169,"context_line":"    else:"},{"line_number":170,"context_line":"        reason \u003d (_(\"Invalid location uri %s\") % uri)"},{"line_number":171,"context_line":"        LOG.warning(reason)"},{"line_number":172,"context_line":"        return"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_aeac0d9f","line":171,"updated":"2019-08-01 16:39:57.000000000","message":"I think on this one, you want reason \u003d _(\"string\") and then LOG.warning(reason, uri) so that the new string won\u0027t be created unless the logger needs it.  Or you can just do LOG.warning(_(\"Invalid location uri %s\", uri)\n\nFeel free to push back on this, I\u0027ve been bad about ignoring unnecessary string creation in my own code, and I\u0027m trying to get better about it, and i may be seeing problems where none exist.","commit_id":"ffb277d97751d334ae4ae5665854495239200861"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"d0015a677ec039622e80429da21851529c5a4ef8","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        return store"},{"line_number":169,"context_line":"    else:"},{"line_number":170,"context_line":"        reason \u003d (_(\"Invalid location uri %s\") % uri)"},{"line_number":171,"context_line":"        LOG.warning(reason)"},{"line_number":172,"context_line":"        return"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_f1c3c8eb","line":171,"in_reply_to":"7faddb67_8e0a7176","updated":"2019-08-01 17:57:38.000000000","message":"Done","commit_id":"ffb277d97751d334ae4ae5665854495239200861"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"85319c8e45a1bc8a06a11cd9e37720dac69d5e7a","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        return store"},{"line_number":169,"context_line":"    else:"},{"line_number":170,"context_line":"        reason \u003d (_(\"Invalid location uri %s\") % uri)"},{"line_number":171,"context_line":"        LOG.warning(reason)"},{"line_number":172,"context_line":"        return"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_8e0a7176","line":171,"in_reply_to":"7faddb67_aeac0d9f","updated":"2019-08-01 16:51:20.000000000","message":"Warning messages should not be translated with _(). And log messages should not be formatted. Get rid of the reason variable, move things into the LOG.warning call, and change \u0027%\u0027 to \u0027,\u0027 so the formatting values are passed in as args to the logger so it can translate when needed.","commit_id":"ffb277d97751d334ae4ae5665854495239200861"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"578e12433492323592bdb0f90ffa8599883cb9ba","unresolved":false,"context_lines":[{"line_number":166,"context_line":"    if url_matched:"},{"line_number":167,"context_line":"        return store"},{"line_number":168,"context_line":"    else:"},{"line_number":169,"context_line":"        LOG.warning(_(\"Invalid location uri %s\"), uri)"},{"line_number":170,"context_line":"        return"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"7faddb67_4948cc5a","line":169,"range":{"start_line":169,"start_character":20,"end_line":169,"end_character":22},"updated":"2019-08-05 12:30:20.000000000","message":"You missed Sean\u0027s comment about not translating log messages.  The current thinking is that all log messages should be untranslated so that operators will get better search engine results.  Only user-facing messages (e.g., exceptions) are translated now. [0]\n\nAlso need to fix this on line 157.\n\n[0] https://docs.openstack.org/oslo.i18n/latest/user/guidelines.html","commit_id":"859b6541ee0a49e569074b18c708985fdc930b5b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"75155b71f13dadd5043bb48da098d6644e502014","unresolved":false,"context_lines":[{"line_number":166,"context_line":"    if url_matched:"},{"line_number":167,"context_line":"        return store"},{"line_number":168,"context_line":"    else:"},{"line_number":169,"context_line":"        LOG.warning(_(\"Invalid location uri %s\"), uri)"},{"line_number":170,"context_line":"        return"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"7faddb67_9781f2fd","line":169,"range":{"start_line":169,"start_character":20,"end_line":169,"end_character":22},"in_reply_to":"7faddb67_4948cc5a","updated":"2019-08-06 05:42:45.000000000","message":"Done","commit_id":"859b6541ee0a49e569074b18c708985fdc930b5b"}],"glance/tests/unit/v2/test_images_resource.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e094bf1b97167b4c5e4f74c1efc621fd9c51fa40","unresolved":false,"context_lines":[{"line_number":4721,"context_line":"    def test_image_lazy_loading_store(self):"},{"line_number":4722,"context_line":"        # assert existing image does not have store in metadata"},{"line_number":4723,"context_line":"        existing_image \u003d self.images[1]"},{"line_number":4724,"context_line":"        self.assertNotIn(\u0027store\u0027, existing_image[\u0027locations\u0027][0][\u0027metadata\u0027])"},{"line_number":4725,"context_line":""},{"line_number":4726,"context_line":"        # assert: store information will be added by lazy loading"},{"line_number":4727,"context_line":"        request \u003d unit_test_utils.get_fake_request()"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_75189d74","line":4724,"range":{"start_line":4724,"start_character":26,"end_line":4724,"end_character":31},"updated":"2019-07-24 18:16:01.000000000","message":"should this be \u0027backend\u0027?","commit_id":"0244da4fd1afe3fb2d4fae966a622a407a06fc1d"}]}