)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"4ccf813c87e6c777b5a3cb620a3c03a7a343cb20","unresolved":false,"context_lines":[{"line_number":12,"context_line":"Made changes to loaction API to findout the store from location URI"},{"line_number":13,"context_line":"and add it as a backend store to the location metadata."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"DocImpact"},{"line_number":16,"context_line":"Closes-Bug: #1802587"},{"line_number":17,"context_line":"Co-Authored-By: Victor Coutellier \u003cvictor.coutellier@gmail.com\u003e"},{"line_number":18,"context_line":"Change-Id: If6d0348346d2086a2500b0012a0e81e80cea7395"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"5faad753_aa32e35c","line":15,"updated":"2019-09-12 14:34:19.000000000","message":"Not sure how we\u0027ve been handling this here, but I just wanted to point out we stopped using DocImpact elsewhere. This triggers the creation of a bug for openstack-manuals. That was useful when there was an actual docs team that would pick them up and incorporate doc changes, but now that that team is pretty much gone and most docs are in-tree, it\u0027s usually easier to not use this and just include the documentation updates with the patch.\n\nThat said, it may still be useful if we need a bug to track any follow on documentation activity that the glance team is going to have to do, so it may still be useful.","commit_id":"4e070fd6e0346948898b751516234ba62121e2a8"}],"glance/api/v2/images.py":[{"author":{"_account_id":28595,"name":"Victor Coutellier","email":"victor.coutellier@gmail.com","username":"alistarle"},"change_message_id":"242967df4901ce61445938ff49234848d5cd52c2","unresolved":false,"context_lines":[{"line_number":451,"context_line":""},{"line_number":452,"context_line":"    def _update_store_in_location(self, image, locations):"},{"line_number":453,"context_line":"        for loc in locations:"},{"line_number":454,"context_line":"            store_id \u003d store_utils.get_store_id_from_uri(loc[\u0027url\u0027])"},{"line_number":455,"context_line":"            if store_id:"},{"line_number":456,"context_line":"                if \u0027store\u0027 in loc[\u0027metadata\u0027]:"},{"line_number":457,"context_line":"                    old_store \u003d loc[\u0027metadata\u0027][\u0027store\u0027]"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_92565d5c","line":454,"updated":"2019-07-10 20:19:34.000000000","message":"Sorry but I really don\u0027t find the function get_store_id_from_uri in the module store_utils, it is in another commit ?","commit_id":"bc55a06fcf6d3c1be14714766498a4300d9e7b0f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"d2d4263bd3cd2167a18f9559b118065b34478a99","unresolved":false,"context_lines":[{"line_number":451,"context_line":""},{"line_number":452,"context_line":"    def _update_store_in_location(self, image, locations):"},{"line_number":453,"context_line":"        for loc in locations:"},{"line_number":454,"context_line":"            store_id \u003d store_utils.get_store_id_from_uri(loc[\u0027url\u0027])"},{"line_number":455,"context_line":"            if store_id:"},{"line_number":456,"context_line":"                if \u0027store\u0027 in loc[\u0027metadata\u0027]:"},{"line_number":457,"context_line":"                    old_store \u003d loc[\u0027metadata\u0027][\u0027store\u0027]"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_83984ac0","line":454,"in_reply_to":"7faddb67_92565d5c","updated":"2019-07-11 04:45:09.000000000","message":"Yes, this is from https://review.opendev.org/#/c/660645/5\n\nThis patch is dependent on above patch.","commit_id":"bc55a06fcf6d3c1be14714766498a4300d9e7b0f"},{"author":{"_account_id":28595,"name":"Victor Coutellier","email":"victor.coutellier@gmail.com","username":"alistarle"},"change_message_id":"0dfeff39d5f634f7de30dd2b4b21ce4ab92849d4","unresolved":false,"context_lines":[{"line_number":453,"context_line":"        for loc in locations:"},{"line_number":454,"context_line":"            store_id \u003d store_utils.get_store_id_from_uri(loc[\u0027url\u0027])"},{"line_number":455,"context_line":"            if store_id:"},{"line_number":456,"context_line":"                if \u0027store\u0027 in loc[\u0027metadata\u0027]:"},{"line_number":457,"context_line":"                    old_store \u003d loc[\u0027metadata\u0027][\u0027store\u0027]"},{"line_number":458,"context_line":"                    if old_store !\u003d store_id:"},{"line_number":459,"context_line":"                        LOG.warning(_(\"Store \u0027%(old)s\u0027 has changed to \""}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_a309c631","line":456,"updated":"2019-07-11 06:37:11.000000000","message":"And what if there is no metadata in the location ? Don\u0027t we need to check that too, I add a test on it, which fail today","commit_id":"bc55a06fcf6d3c1be14714766498a4300d9e7b0f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"1f9ffc32e0391361ac73a75411409f2fd85dbdd4","unresolved":false,"context_lines":[{"line_number":453,"context_line":"        for loc in locations:"},{"line_number":454,"context_line":"            store_id \u003d store_utils.get_store_id_from_uri(loc[\u0027url\u0027])"},{"line_number":455,"context_line":"            if store_id:"},{"line_number":456,"context_line":"                if \u0027store\u0027 in loc[\u0027metadata\u0027]:"},{"line_number":457,"context_line":"                    old_store \u003d loc[\u0027metadata\u0027][\u0027store\u0027]"},{"line_number":458,"context_line":"                    if old_store !\u003d store_id:"},{"line_number":459,"context_line":"                        LOG.warning(_(\"Store \u0027%(old)s\u0027 has changed to \""}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_46e7c041","line":456,"in_reply_to":"7faddb67_a309c631","updated":"2019-07-11 06:48:58.000000000","message":"there will always be metadata in the location with empty dictionary associated with it.","commit_id":"bc55a06fcf6d3c1be14714766498a4300d9e7b0f"},{"author":{"_account_id":23561,"name":"iain MacDonnell","email":"iain.macdonnell@oracle.com","username":"imacdonn"},"change_message_id":"586b45c162c609fa873f5ffed87f574b77fffa79","unresolved":false,"context_lines":[{"line_number":465,"context_line":"                            \u0027id\u0027: image.image_id})"},{"line_number":466,"context_line":"                    else:"},{"line_number":467,"context_line":"                        continue"},{"line_number":468,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_id"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def _do_replace_locations(self, image, value):"},{"line_number":471,"context_line":"        if CONF.show_multiple_locations \u003d\u003d False:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_15421369","line":468,"updated":"2019-06-26 17:23:01.000000000","message":"Isn\u0027t this going to override the user-provided store selection every time? Doesn\u0027t this completely defeat the purpose of allowing the user to specify a store in their request? Shouldn\u0027t this change only populate the store if it is not already populated?","commit_id":"bc55a06fcf6d3c1be14714766498a4300d9e7b0f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"002556f1650bd03ce55b62feda902da82664edb5","unresolved":false,"context_lines":[{"line_number":465,"context_line":"                            \u0027id\u0027: image.image_id})"},{"line_number":466,"context_line":"                    else:"},{"line_number":467,"context_line":"                        continue"},{"line_number":468,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_id"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def _do_replace_locations(self, image, value):"},{"line_number":471,"context_line":"        if CONF.show_multiple_locations \u003d\u003d False:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_d3e24130","line":468,"in_reply_to":"9fb8cfa7_15421369","updated":"2019-06-27 14:01:17.000000000","message":"Ian look at condition at line #456, if store is not in location then only it will add store to location metadata at line #468.","commit_id":"bc55a06fcf6d3c1be14714766498a4300d9e7b0f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"676ab333ae5b05f3d93c0ecace6b9e4025777ef9","unresolved":false,"context_lines":[{"line_number":465,"context_line":"                            \u0027id\u0027: image.image_id})"},{"line_number":466,"context_line":"                    else:"},{"line_number":467,"context_line":"                        continue"},{"line_number":468,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_id"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def _do_replace_locations(self, image, value):"},{"line_number":471,"context_line":"        if CONF.show_multiple_locations \u003d\u003d False:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_2e8c8f34","line":468,"in_reply_to":"9fb8cfa7_4e0a23d2","updated":"2019-06-27 18:20:46.000000000","message":"We have store id defined in config file and there is a possibility that ad min/operator can change the store names. In that case this 456 line will check if location has store and if new location based on url is different then it will replace old one with new else it will just continue and if store is not in metadata at line 456 then it will simply add the store to metadata at line 468.","commit_id":"bc55a06fcf6d3c1be14714766498a4300d9e7b0f"},{"author":{"_account_id":23561,"name":"iain MacDonnell","email":"iain.macdonnell@oracle.com","username":"imacdonn"},"change_message_id":"7e3ee00d199c0e7047699106dad73e2c0905e7fc","unresolved":false,"context_lines":[{"line_number":465,"context_line":"                            \u0027id\u0027: image.image_id})"},{"line_number":466,"context_line":"                    else:"},{"line_number":467,"context_line":"                        continue"},{"line_number":468,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_id"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def _do_replace_locations(self, image, value):"},{"line_number":471,"context_line":"        if CONF.show_multiple_locations \u003d\u003d False:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_4e0a23d2","line":468,"in_reply_to":"9fb8cfa7_d3e24130","updated":"2019-06-27 18:10:12.000000000","message":"Maybe I\u0027m being stupid, but doesn\u0027t like 456 do the exact opposite of that? There is no \"not\" in that line....","commit_id":"bc55a06fcf6d3c1be14714766498a4300d9e7b0f"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"3e47e42b80dcddb3367f76ab3d042716dd41ae8f","unresolved":false,"context_lines":[{"line_number":456,"context_line":"                if \u0027store\u0027 in loc[\u0027metadata\u0027]:"},{"line_number":457,"context_line":"                    old_store \u003d loc[\u0027metadata\u0027][\u0027store\u0027]"},{"line_number":458,"context_line":"                    if old_store !\u003d store_id:"},{"line_number":459,"context_line":"                        LOG.warning(_(\"Store \u0027%(old)s\u0027 has changed to \""},{"line_number":460,"context_line":"                                      \"\u0027%(new)s\u0027 by operator, updating \""},{"line_number":461,"context_line":"                                      \"the same in the location of image \""},{"line_number":462,"context_line":"                                      \"\u0027%(id)s\u0027\") % {"},{"line_number":463,"context_line":"                            \u0027old\u0027: old_store,"},{"line_number":464,"context_line":"                            \u0027new\u0027: store_id,"},{"line_number":465,"context_line":"                            \u0027id\u0027: image.image_id})"},{"line_number":466,"context_line":"                    else:"},{"line_number":467,"context_line":"                        continue"},{"line_number":468,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_id"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_0216c2f2","line":465,"range":{"start_line":459,"start_character":0,"end_line":465,"end_character":50},"updated":"2019-07-25 11:29:13.000000000","message":"Is there specific reason why this is logging as warning? If I understand this correctly, this is expected behaviour of the operation and will flood the logs with warnings that are not exactly something operator should take action on.\n\nIMHO this should be debug or at very most info.","commit_id":"70a543acbff841bbb34b545df8282f0c277ab043"}],"glance/tests/unit/common/test_utils.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"600a0bf49b6118a979237c1db67c73501640e04f","unresolved":false,"context_lines":[{"line_number":37,"context_line":"            \u0027metadata\u0027: metadata"},{"line_number":38,"context_line":"        }]"},{"line_number":39,"context_line":"        with mock.patch.object("},{"line_number":40,"context_line":"                store_utils, \u0027_get_store_id_from_uri\u0027) as mock_get_store_id:"},{"line_number":41,"context_line":"            mock_get_store_id.return_value \u003d store_id"},{"line_number":42,"context_line":"            store_utils.update_store_in_locations(locations, mock.Mock())"},{"line_number":43,"context_line":"            self.assertEqual(locations[0][\u0027metadata\u0027][\u0027store\u0027], expected)"}],"source_content_type":"text/x-python","patch_set":14,"id":"7faddb67_eabe9f63","line":40,"range":{"start_line":40,"start_character":30,"end_line":40,"end_character":52},"updated":"2019-07-30 14:10:01.000000000","message":"This can also return None.  I think it would be worth adding a test to make sure nothing blows up in that case.","commit_id":"a9ea77f7c566ff3966926c22b626a3f1e6db4eab"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"38fdf6fa385800160f17c20e246f5f5e19faf77f","unresolved":false,"context_lines":[{"line_number":37,"context_line":"            \u0027metadata\u0027: metadata"},{"line_number":38,"context_line":"        }]"},{"line_number":39,"context_line":"        with mock.patch.object("},{"line_number":40,"context_line":"                store_utils, \u0027_get_store_id_from_uri\u0027) as mock_get_store_id:"},{"line_number":41,"context_line":"            mock_get_store_id.return_value \u003d store_id"},{"line_number":42,"context_line":"            store_utils.update_store_in_locations(locations, mock.Mock())"},{"line_number":43,"context_line":"            self.assertEqual(locations[0][\u0027metadata\u0027][\u0027store\u0027], expected)"}],"source_content_type":"text/x-python","patch_set":14,"id":"7faddb67_eb1a217b","line":40,"range":{"start_line":40,"start_character":30,"end_line":40,"end_character":52},"in_reply_to":"7faddb67_eabe9f63","updated":"2019-07-30 16:56:41.000000000","message":"Done","commit_id":"a9ea77f7c566ff3966926c22b626a3f1e6db4eab"}],"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":"600a0bf49b6118a979237c1db67c73501640e04f","unresolved":false,"context_lines":[{"line_number":1973,"context_line":"                        name\u003d\u00271\u0027,"},{"line_number":1974,"context_line":"                        disk_format\u003d\u0027raw\u0027,"},{"line_number":1975,"context_line":"                        container_format\u003d\u0027bare\u0027,"},{"line_number":1976,"context_line":"                        status\u003d\u0027queued\u0027),"},{"line_number":1977,"context_line":"        ]"},{"line_number":1978,"context_line":"        self.db.image_create(None, self.images[0])"},{"line_number":1979,"context_line":"        request \u003d unit_test_utils.get_fake_request()"}],"source_content_type":"text/x-python","patch_set":14,"id":"7faddb67_a5d9b856","line":1976,"updated":"2019-07-30 14:10:01.000000000","message":"I wonder whether you should add some locations here:\n\nloc1 \u003d {\u0027url\u0027: \u0027unknown://whocares\u0027, \u0027metadata\u0027: {}}\nloc2 \u003d {\u0027url\u0027: \u0027unknown://whatever\u0027, \u0027metadata\u0027:{\u0027store\u0027: \u0027unkstore\u0027}}\nlocations\u003d[loc1, loc2]\n\n(I guess the status would have to change to \u0027active\u0027 in that case, so maybe a different test if you want to make sure the transition from queued-\u003eactive is being done correctly as you\u0027re checking at line 1991)\n\nThe point would be to make sure that the lazy update doesn\u0027t barf on un-enabled stores, and also that it doesn\u0027t modify the locations when it doesn\u0027t recognize the URL.  I think this is a real use case--I can see an operator removing a store from enabled_stores temporarily for maintenance or something, but still wanting the locations to work when the store gets added back to enabled_backends.\n\nSorry to be a PITA, but my worry is that if the code had a bug where it rewrote the \u0027store\u0027 in all locations to be \u0027fake-store\u0027, I don\u0027t think this test would catch that.","commit_id":"a9ea77f7c566ff3966926c22b626a3f1e6db4eab"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"38fdf6fa385800160f17c20e246f5f5e19faf77f","unresolved":false,"context_lines":[{"line_number":1973,"context_line":"                        name\u003d\u00271\u0027,"},{"line_number":1974,"context_line":"                        disk_format\u003d\u0027raw\u0027,"},{"line_number":1975,"context_line":"                        container_format\u003d\u0027bare\u0027,"},{"line_number":1976,"context_line":"                        status\u003d\u0027queued\u0027),"},{"line_number":1977,"context_line":"        ]"},{"line_number":1978,"context_line":"        self.db.image_create(None, self.images[0])"},{"line_number":1979,"context_line":"        request \u003d unit_test_utils.get_fake_request()"}],"source_content_type":"text/x-python","patch_set":14,"id":"7faddb67_c0a13ff9","line":1976,"in_reply_to":"7faddb67_a5d9b856","updated":"2019-07-30 16:56:41.000000000","message":"Done","commit_id":"a9ea77f7c566ff3966926c22b626a3f1e6db4eab"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"be3e25fdaa80b5a3c5c363ef356ecfdd98ea35ce","unresolved":false,"context_lines":[{"line_number":1841,"context_line":"    @mock.patch.object(glance.location.ImageRepoProxy, \u0027_set_acls\u0027)"},{"line_number":1842,"context_line":"    @mock.patch.object(store, \u0027get_size_from_uri_and_backend\u0027)"},{"line_number":1843,"context_line":"    @mock.patch.object(store, \u0027get_size_from_backend\u0027)"},{"line_number":1844,"context_line":"    def test_replace_locations_unknon_locations("},{"line_number":1845,"context_line":"            self, mock_get_size, mock_get_size_uri, mock_set_acls,"},{"line_number":1846,"context_line":"            mock_check_loc, mock_calc):"},{"line_number":1847,"context_line":"        mock_calc.return_value \u003d 1"}],"source_content_type":"text/x-python","patch_set":17,"id":"7faddb67_e97e5868","line":1844,"range":{"start_line":1844,"start_character":31,"end_line":1844,"end_character":37},"updated":"2019-08-05 12:51:01.000000000","message":"Nit: only if you need to put up another patch -- \u0027unknown\u0027 (the Glance code policy disallows corrections, so if this is merged, it will be here forever!)\n\nAnother nit: might be worth either changing the name of the test (and the one below) or adding a comment to make it clear that we\u0027re only testing the store-metadata-lazy-update code with these tests.","commit_id":"85576902a6bb496bc4182f1f1b21232ae02ba339"}]}