)]}'
{"glance/common/store_utils.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4b27dc242c562e77f9625abcffe73d2e982526ac","unresolved":false,"context_lines":[{"line_number":212,"context_line":"    scheme \u003d urlparse.urlparse(uri).scheme"},{"line_number":213,"context_line":"    location_map \u003d store_api.location.SCHEME_TO_CLS_BACKEND_MAP"},{"line_number":214,"context_line":"    if scheme not in location_map:"},{"line_number":215,"context_line":"        LOG.warning(\"Unknown scheme \u0027%(scheme)s\u0027 found in uri \u0027%(uri)s\u0027\", {"},{"line_number":216,"context_line":"            \u0027scheme\u0027: scheme, \u0027uri\u0027: uri})"},{"line_number":217,"context_line":"        return"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    for store in location_map[scheme]:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_cd519610","line":216,"range":{"start_line":215,"start_character":8,"end_line":216,"end_character":42},"updated":"2020-08-26 06:09:06.000000000","message":"Need to use translation here","commit_id":"7f6bbe4ec836bc8af9ba58fd48466715ada018e0"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6f81f624a0221a5ebf6853ae16d86f03a4a78f3e","unresolved":false,"context_lines":[{"line_number":212,"context_line":"    scheme \u003d urlparse.urlparse(uri).scheme"},{"line_number":213,"context_line":"    location_map \u003d store_api.location.SCHEME_TO_CLS_BACKEND_MAP"},{"line_number":214,"context_line":"    if scheme not in location_map:"},{"line_number":215,"context_line":"        LOG.warning(\"Unknown scheme \u0027%(scheme)s\u0027 found in uri \u0027%(uri)s\u0027\", {"},{"line_number":216,"context_line":"            \u0027scheme\u0027: scheme, \u0027uri\u0027: uri})"},{"line_number":217,"context_line":"        return"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    for store in location_map[scheme]:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_838acf60","line":216,"range":{"start_line":215,"start_character":8,"end_line":216,"end_character":42},"in_reply_to":"9f560f44_cd519610","updated":"2020-08-26 08:34:35.000000000","message":"Done","commit_id":"7f6bbe4ec836bc8af9ba58fd48466715ada018e0"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4b27dc242c562e77f9625abcffe73d2e982526ac","unresolved":false,"context_lines":[{"line_number":224,"context_line":"            loc[\u0027metadata\u0027][\u0027store\u0027] \u003d u\"%s\" % store"},{"line_number":225,"context_line":"            return"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    LOG.warning(\"Not able to update location url \u0027%s\u0027 of legacy image due to \""},{"line_number":228,"context_line":"                \"unknown issues.\", uri)"},{"line_number":229,"context_line":"    return"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_8d2e7e89","line":228,"range":{"start_line":227,"start_character":4,"end_line":228,"end_character":39},"updated":"2020-08-26 06:09:06.000000000","message":"You need to use translation for this,","commit_id":"7f6bbe4ec836bc8af9ba58fd48466715ada018e0"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6f81f624a0221a5ebf6853ae16d86f03a4a78f3e","unresolved":false,"context_lines":[{"line_number":224,"context_line":"            loc[\u0027metadata\u0027][\u0027store\u0027] \u003d u\"%s\" % store"},{"line_number":225,"context_line":"            return"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    LOG.warning(\"Not able to update location url \u0027%s\u0027 of legacy image due to \""},{"line_number":228,"context_line":"                \"unknown issues.\", uri)"},{"line_number":229,"context_line":"    return"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_e3eaab0c","line":228,"range":{"start_line":227,"start_character":4,"end_line":228,"end_character":39},"in_reply_to":"9f560f44_8d2e7e89","updated":"2020-08-26 08:34:35.000000000","message":"Done","commit_id":"7f6bbe4ec836bc8af9ba58fd48466715ada018e0"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4b27dc242c562e77f9625abcffe73d2e982526ac","unresolved":false,"context_lines":[{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    LOG.warning(\"Not able to update location url \u0027%s\u0027 of legacy image due to \""},{"line_number":228,"context_line":"                \"unknown issues.\", uri)"},{"line_number":229,"context_line":"    return"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"def get_updated_store_location(locations):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_2d55b2fa","line":229,"range":{"start_line":229,"start_character":4,"end_line":229,"end_character":10},"updated":"2020-08-26 06:09:06.000000000","message":"No need of this","commit_id":"7f6bbe4ec836bc8af9ba58fd48466715ada018e0"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6f81f624a0221a5ebf6853ae16d86f03a4a78f3e","unresolved":false,"context_lines":[{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    LOG.warning(\"Not able to update location url \u0027%s\u0027 of legacy image due to \""},{"line_number":228,"context_line":"                \"unknown issues.\", uri)"},{"line_number":229,"context_line":"    return"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"def get_updated_store_location(locations):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_c3ede725","line":229,"range":{"start_line":229,"start_character":4,"end_line":229,"end_character":10},"in_reply_to":"9f560f44_2d55b2fa","updated":"2020-08-26 08:34:35.000000000","message":"Done","commit_id":"7f6bbe4ec836bc8af9ba58fd48466715ada018e0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3304315731ca0556cad26328204362b619caf50e","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        if (not loc[\u0027metadata\u0027].get("},{"line_number":185,"context_line":"                \u0027store\u0027) or loc[\u0027metadata\u0027].get("},{"line_number":186,"context_line":"                \u0027store\u0027) not in CONF.enabled_backends):"},{"line_number":187,"context_line":"            if loc[\u0027url\u0027].startswith(\"cinder://\"):"},{"line_number":188,"context_line":"                _update_cinder_location_and_store_id(context, loc)"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"            store_id \u003d _get_store_id_from_uri(loc[\u0027url\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_71deee3e","line":187,"updated":"2020-09-02 14:50:41.000000000","message":"Based on your reno, it looks like this will continue to match every image even after they have been migrated, which isn\u0027t what you want right? We don\u0027t want to have to rewrite the locations all the time, only if they\u0027re old. Maybe we have a test function that looks to see if the thing after the scheme has one slash (new) or no slashes (old) ?","commit_id":"fd0c6f453cb9c588c69b7f94df94165125651d94"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"822ce54f62f838750535e32e649ca055ada73d3b","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        if (not loc[\u0027metadata\u0027].get("},{"line_number":185,"context_line":"                \u0027store\u0027) or loc[\u0027metadata\u0027].get("},{"line_number":186,"context_line":"                \u0027store\u0027) not in CONF.enabled_backends):"},{"line_number":187,"context_line":"            if loc[\u0027url\u0027].startswith(\"cinder://\"):"},{"line_number":188,"context_line":"                _update_cinder_location_and_store_id(context, loc)"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"            store_id \u003d _get_store_id_from_uri(loc[\u0027url\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_5f04847e","line":187,"in_reply_to":"9f560f44_31f1166c","updated":"2020-09-04 19:13:25.000000000","message":"Adding to what Abhishek said, the metadata will be updated once and this line will never be reached.\nThis has been confirmed in my manual testing.","commit_id":"fd0c6f453cb9c588c69b7f94df94165125651d94"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3394584297aeed81d25bda8a3b72da0d99d454bd","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        if (not loc[\u0027metadata\u0027].get("},{"line_number":185,"context_line":"                \u0027store\u0027) or loc[\u0027metadata\u0027].get("},{"line_number":186,"context_line":"                \u0027store\u0027) not in CONF.enabled_backends):"},{"line_number":187,"context_line":"            if loc[\u0027url\u0027].startswith(\"cinder://\"):"},{"line_number":188,"context_line":"                _update_cinder_location_and_store_id(context, loc)"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"            store_id \u003d _get_store_id_from_uri(loc[\u0027url\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_31f1166c","line":187,"in_reply_to":"9f560f44_71deee3e","updated":"2020-09-02 15:10:05.000000000","message":"this will be checked in above condition only at line #184","commit_id":"fd0c6f453cb9c588c69b7f94df94165125651d94"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3304315731ca0556cad26328204362b619caf50e","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        if store_instance.is_image_associated_with_store(context, volume_id):"},{"line_number":222,"context_line":"            url_prefix \u003d store_instance.url_prefix"},{"line_number":223,"context_line":"            loc[\u0027url\u0027] \u003d \"%s/%s\" % (url_prefix, volume_id)"},{"line_number":224,"context_line":"            loc[\u0027metadata\u0027][\u0027store\u0027] \u003d u\"%s\" % store"},{"line_number":225,"context_line":"            return"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    LOG.warning(_LW(\"Not able to update location url \u0027%s\u0027 of legacy image \""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_8d1ba87e","line":224,"range":{"start_line":224,"start_character":39,"end_line":224,"end_character":40},"updated":"2020-09-02 14:50:41.000000000","message":"This shouldn\u0027t be a thing anymore in a python3-only world, right?","commit_id":"fd0c6f453cb9c588c69b7f94df94165125651d94"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3394584297aeed81d25bda8a3b72da0d99d454bd","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        if store_instance.is_image_associated_with_store(context, volume_id):"},{"line_number":222,"context_line":"            url_prefix \u003d store_instance.url_prefix"},{"line_number":223,"context_line":"            loc[\u0027url\u0027] \u003d \"%s/%s\" % (url_prefix, volume_id)"},{"line_number":224,"context_line":"            loc[\u0027metadata\u0027][\u0027store\u0027] \u003d u\"%s\" % store"},{"line_number":225,"context_line":"            return"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    LOG.warning(_LW(\"Not able to update location url \u0027%s\u0027 of legacy image \""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_f1f67e5d","line":224,"range":{"start_line":224,"start_character":39,"end_line":224,"end_character":40},"in_reply_to":"9f560f44_8d1ba87e","updated":"2020-09-02 15:10:05.000000000","message":"shouldn\u0027t be, I think he added it to just match with glance_store add method \n\nhttps://github.com/openstack/glance_store/blob/master/glance_store/_drivers/cinder.py#L930\nhttps://github.com/openstack/glance_store/blob/master/glance_store/_drivers/filesystem.py#L797\nhttps://github.com/openstack/glance_store/blob/master/glance_store/_drivers/rbd.py#L639","commit_id":"fd0c6f453cb9c588c69b7f94df94165125651d94"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"822ce54f62f838750535e32e649ca055ada73d3b","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        if store_instance.is_image_associated_with_store(context, volume_id):"},{"line_number":222,"context_line":"            url_prefix \u003d store_instance.url_prefix"},{"line_number":223,"context_line":"            loc[\u0027url\u0027] \u003d \"%s/%s\" % (url_prefix, volume_id)"},{"line_number":224,"context_line":"            loc[\u0027metadata\u0027][\u0027store\u0027] \u003d u\"%s\" % store"},{"line_number":225,"context_line":"            return"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    LOG.warning(_LW(\"Not able to update location url \u0027%s\u0027 of legacy image \""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_bb35d4aa","line":224,"range":{"start_line":224,"start_character":39,"end_line":224,"end_character":40},"in_reply_to":"9f560f44_f1f67e5d","updated":"2020-09-04 19:13:25.000000000","message":"Yes, python3 default is unicode and not str.\nremoving it in next PS.","commit_id":"fd0c6f453cb9c588c69b7f94df94165125651d94"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3304315731ca0556cad26328204362b619caf50e","unresolved":false,"context_lines":[{"line_number":225,"context_line":"            return"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    LOG.warning(_LW(\"Not able to update location url \u0027%s\u0027 of legacy image \""},{"line_number":228,"context_line":"                \"due to unknown issues.\"), uri)"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"def get_updated_store_location(locations):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_91074219","line":228,"updated":"2020-09-02 14:50:41.000000000","message":"nit: this line should be indented to align with the opening quote on the previous line.","commit_id":"fd0c6f453cb9c588c69b7f94df94165125651d94"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"822ce54f62f838750535e32e649ca055ada73d3b","unresolved":false,"context_lines":[{"line_number":225,"context_line":"            return"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    LOG.warning(_LW(\"Not able to update location url \u0027%s\u0027 of legacy image \""},{"line_number":228,"context_line":"                \"due to unknown issues.\"), uri)"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"def get_updated_store_location(locations):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_3b49c431","line":228,"in_reply_to":"9f560f44_91074219","updated":"2020-09-04 19:13:25.000000000","message":"Done","commit_id":"fd0c6f453cb9c588c69b7f94df94165125651d94"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d685f0cc68a8d263e89c88fcdaf573fd27b6948","unresolved":false,"context_lines":[{"line_number":206,"context_line":"        image_repo.save(image)"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"def _update_cinder_location_and_store_id(context, loc):"},{"line_number":210,"context_line":"    uri \u003d loc[\u0027url\u0027]"},{"line_number":211,"context_line":"    volume_id \u003d loc[\u0027url\u0027].split(\"/\")[-1]"},{"line_number":212,"context_line":"    scheme \u003d urlparse.urlparse(uri).scheme"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_8a519b11","line":209,"updated":"2020-09-04 21:19:53.000000000","message":"A docstring on this would go a long way to clarifying the thing we\u0027ve been iterating on here. Maybe you can take words from my suggested comments on the test to fill this out and make it clearer?","commit_id":"df19b8cf75c7413a6090cc628b55160c4371694a"}],"glance/tests/functional/__init__.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d60f39b6dfa1ee312802c27e85079301cf4fa46","unresolved":false,"context_lines":[{"line_number":1738,"context_line":""},{"line_number":1739,"context_line":"        self.assertEqual(\u0027active\u0027, image[\u0027status\u0027])"},{"line_number":1740,"context_line":""},{"line_number":1741,"context_line":"        return image_id"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_9b90d044","line":1741,"updated":"2020-09-04 19:43:42.000000000","message":"It would be nice if this refactor was in a different patch. If we had to backport another thing that requires changes to that import locking test, then we\u0027d either have to customize something for stable, or backport your patch/feature which wouldn\u0027t be ideal.","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac26c73405468c3125b550fe7160b4975d496b1a","unresolved":false,"context_lines":[{"line_number":1738,"context_line":""},{"line_number":1739,"context_line":"        self.assertEqual(\u0027active\u0027, image[\u0027status\u0027])"},{"line_number":1740,"context_line":""},{"line_number":1741,"context_line":"        return image_id"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_e704d216","line":1741,"in_reply_to":"9f560f44_9b90d044","updated":"2020-09-04 20:48:21.000000000","message":"Done","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"}],"glance/tests/functional/v2/test_legacy_update_cinder_store.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d60f39b6dfa1ee312802c27e85079301cf4fa46","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        self.start_server()"},{"line_number":112,"context_line":"        vol_id \u003d uuid.uuid4()"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        mocked_cc.return_value \u003d FakeObject("},{"line_number":115,"context_line":"            client\u003dmock.MagicMock(), volumes\u003dFakeObject("},{"line_number":116,"context_line":"                detach\u003dmock.MagicMock(),"},{"line_number":117,"context_line":"                create\u003dlambda size_gb, name, metadata, volume_type:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_9bb9f0cc","line":114,"range":{"start_line":114,"start_character":33,"end_line":114,"end_character":43},"updated":"2020-09-04 19:43:42.000000000","message":"Why not MagicMock ? It should behave the same.","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a5a2519dc4b80fea7817f2a40f8421c290d8d0ad","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        self.start_server()"},{"line_number":112,"context_line":"        vol_id \u003d uuid.uuid4()"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        mocked_cc.return_value \u003d FakeObject("},{"line_number":115,"context_line":"            client\u003dmock.MagicMock(), volumes\u003dFakeObject("},{"line_number":116,"context_line":"                detach\u003dmock.MagicMock(),"},{"line_number":117,"context_line":"                create\u003dlambda size_gb, name, metadata, volume_type:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_674ba2b0","line":114,"range":{"start_line":114,"start_character":33,"end_line":114,"end_character":43},"in_reply_to":"9f560f44_1b7040da","updated":"2020-09-04 20:59:47.000000000","message":"You would do:\n\ncinderclient.volumes.create.return_value.manager.get.return_value.status.return_value \u003d \u0027in-use\u0027","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac26c73405468c3125b550fe7160b4975d496b1a","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        self.start_server()"},{"line_number":112,"context_line":"        vol_id \u003d uuid.uuid4()"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        mocked_cc.return_value \u003d FakeObject("},{"line_number":115,"context_line":"            client\u003dmock.MagicMock(), volumes\u003dFakeObject("},{"line_number":116,"context_line":"                detach\u003dmock.MagicMock(),"},{"line_number":117,"context_line":"                create\u003dlambda size_gb, name, metadata, volume_type:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_1b7040da","line":114,"range":{"start_line":114,"start_character":33,"end_line":114,"end_character":43},"in_reply_to":"9f560f44_9bb9f0cc","updated":"2020-09-04 20:48:21.000000000","message":"I didn\u0027t find a good way in mock to assign return values for multiple layers of objects and methods\nEg: cinderclient.volumes.create().manager.get().status.return_value \u003d \u0027in-use\u0027","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d60f39b6dfa1ee312802c27e85079301cf4fa46","unresolved":false,"context_lines":[{"line_number":179,"context_line":"                            update_all_metadata\u003dmock.MagicMock(),"},{"line_number":180,"context_line":"                            update_readonly_flag\u003dmock.MagicMock())))),"},{"line_number":181,"context_line":"            volume_types\u003dFakeObject("},{"line_number":182,"context_line":"                default\u003dlambda: FakeObject(name\u003d\u0027some_type\u0027)))"},{"line_number":183,"context_line":"        # create image in single store"},{"line_number":184,"context_line":"        image_id \u003d self._create_and_import(stores\u003d[\u0027store1\u0027])"},{"line_number":185,"context_line":"        image \u003d self.api_get(\u0027/v2/images/%s\u0027 % image_id).json"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_1bc5e03d","line":182,"updated":"2020-09-04 19:43:42.000000000","message":"It\u0027s hard to tell - is this the same as the object you have in the first test? If so, probably better to create this at setup time and use the same thing in both places. If they\u0027re not the same, it\u0027d be ideal to re-use as much of the structure as you can and only tweak the different bits in these tests so we can easily see what the difference is.","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac26c73405468c3125b550fe7160b4975d496b1a","unresolved":false,"context_lines":[{"line_number":179,"context_line":"                            update_all_metadata\u003dmock.MagicMock(),"},{"line_number":180,"context_line":"                            update_readonly_flag\u003dmock.MagicMock())))),"},{"line_number":181,"context_line":"            volume_types\u003dFakeObject("},{"line_number":182,"context_line":"                default\u003dlambda: FakeObject(name\u003d\u0027some_type\u0027)))"},{"line_number":183,"context_line":"        # create image in single store"},{"line_number":184,"context_line":"        image_id \u003d self._create_and_import(stores\u003d[\u0027store1\u0027])"},{"line_number":185,"context_line":"        image \u003d self.api_get(\u0027/v2/images/%s\u0027 % image_id).json"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_47b13e44","line":182,"in_reply_to":"9f560f44_1bc5e03d","updated":"2020-09-04 20:48:21.000000000","message":"Done","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d60f39b6dfa1ee312802c27e85079301cf4fa46","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        self.assertEqual(\u0027cinder://store1/%s\u0027 % vol_id,"},{"line_number":196,"context_line":"                         image[\u0027locations\u0027][0][\u0027url\u0027])"},{"line_number":197,"context_line":"        self.assertEqual(\u0027store1\u0027, image[\u0027locations\u0027][0][\u0027metadata\u0027][\u0027store\u0027])"},{"line_number":198,"context_line":"        # get the image again to see the url is only updated once"},{"line_number":199,"context_line":"        image \u003d self.api_get(\u0027/v2/images/%s\u0027 % image_id).json"},{"line_number":200,"context_line":"        # verify the image location url stays same"},{"line_number":201,"context_line":"        self.assertEqual(\u0027cinder://store1/%s\u0027 % vol_id,"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_bbdb141f","line":198,"updated":"2020-09-04 19:43:42.000000000","message":"This is good, but does it really ensure that we didn\u0027t try to do the migration again? I think it doesn\u0027t, so I would make this comment something like \"get the image again to see the url is consistent\", or just remove it since you have the comment on L200.","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac26c73405468c3125b550fe7160b4975d496b1a","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        self.assertEqual(\u0027cinder://store1/%s\u0027 % vol_id,"},{"line_number":196,"context_line":"                         image[\u0027locations\u0027][0][\u0027url\u0027])"},{"line_number":197,"context_line":"        self.assertEqual(\u0027store1\u0027, image[\u0027locations\u0027][0][\u0027metadata\u0027][\u0027store\u0027])"},{"line_number":198,"context_line":"        # get the image again to see the url is only updated once"},{"line_number":199,"context_line":"        image \u003d self.api_get(\u0027/v2/images/%s\u0027 % image_id).json"},{"line_number":200,"context_line":"        # verify the image location url stays same"},{"line_number":201,"context_line":"        self.assertEqual(\u0027cinder://store1/%s\u0027 % vol_id,"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_87ed1627","line":198,"in_reply_to":"9f560f44_bbdb141f","updated":"2020-09-04 20:48:21.000000000","message":"Done","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a5a2519dc4b80fea7817f2a40f8421c290d8d0ad","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def setUp(self):"},{"line_number":44,"context_line":"        super(TestLegacyUpdateCinderStore, self).setUp()"},{"line_number":45,"context_line":"        self.vol_id \u003d uuid.uuid4()"},{"line_number":46,"context_line":"        self.cinder_store_mock \u003d FakeObject("},{"line_number":47,"context_line":"            client\u003dmock.MagicMock(), volumes\u003dFakeObject("},{"line_number":48,"context_line":"                get\u003dlambda v_id: FakeObject(volume_type\u003d\u0027fast\u0027),"},{"line_number":49,"context_line":"                detach\u003dmock.MagicMock(),"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_c7d1cece","line":46,"range":{"start_line":46,"start_character":33,"end_line":46,"end_character":43},"updated":"2020-09-04 20:59:47.000000000","message":"Still think this should be all MagicMocks, but we could fix that up later if the syntax is the only hangup.","commit_id":"df19b8cf75c7413a6090cc628b55160c4371694a"}],"glance/tests/unit/common/test_utils.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3304315731ca0556cad26328204362b619caf50e","unresolved":false,"context_lines":[{"line_number":143,"context_line":"                                                   is_valid\u003dFalse,"},{"line_number":144,"context_line":"                                                   save_call_count\u003d0)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"class TestUtils(test_utils.BaseTestCase):"},{"line_number":148,"context_line":"    \"\"\"Test routines in glance.utils\"\"\""},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_11efb2e9","line":146,"updated":"2020-09-02 14:50:41.000000000","message":"I think you need a third case here where we run this again after migration and make sure that the locations are not re-migrated. Alternately, it might be best to cover this case via a functional test.","commit_id":"fd0c6f453cb9c588c69b7f94df94165125651d94"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4e09d79797dc782f597d5d594b941c1417f9a9ac","unresolved":false,"context_lines":[{"line_number":143,"context_line":"                                                   is_valid\u003dFalse,"},{"line_number":144,"context_line":"                                                   save_call_count\u003d0)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"class TestUtils(test_utils.BaseTestCase):"},{"line_number":148,"context_line":"    \"\"\"Test routines in glance.utils\"\"\""},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_304c5a75","line":146,"in_reply_to":"9f560f44_11efb2e9","updated":"2020-09-03 14:44:40.000000000","message":"We could have a functional test which will create image in cinder store and then will call get API and then we can make sure location has not changed or we can create image in file store, copy it to cinder store and then call get API to see location is not changed.","commit_id":"fd0c6f453cb9c588c69b7f94df94165125651d94"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"822ce54f62f838750535e32e649ca055ada73d3b","unresolved":false,"context_lines":[{"line_number":143,"context_line":"                                                   is_valid\u003dFalse,"},{"line_number":144,"context_line":"                                                   save_call_count\u003d0)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"class TestUtils(test_utils.BaseTestCase):"},{"line_number":148,"context_line":"    \"\"\"Test routines in glance.utils\"\"\""},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_5b4e3838","line":146,"in_reply_to":"9f560f44_304c5a75","updated":"2020-09-04 19:13:25.000000000","message":"Done","commit_id":"fd0c6f453cb9c588c69b7f94df94165125651d94"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d60f39b6dfa1ee312802c27e85079301cf4fa46","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        if is_valid:"},{"line_number":121,"context_line":"            expected_url \u003d mock_url_prefix.return_value + \u0027/\u0027 + volume_id"},{"line_number":122,"context_line":"        else:"},{"line_number":123,"context_line":"            expected_url \u003d locations[0][\u0027url\u0027]"},{"line_number":124,"context_line":"        image.locations \u003d locations"},{"line_number":125,"context_line":"        store_utils.update_store_in_locations(context, image, image_repo)"},{"line_number":126,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_fbcf4c26","line":123,"updated":"2020-09-04 19:43:42.000000000","message":"In the !is_valid case, we expect it to be migrated already right? So shouldn\u0027t expected_url still be the same as the expected case when we *did* do the migration?","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac26c73405468c3125b550fe7160b4975d496b1a","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        if is_valid:"},{"line_number":121,"context_line":"            expected_url \u003d mock_url_prefix.return_value + \u0027/\u0027 + volume_id"},{"line_number":122,"context_line":"        else:"},{"line_number":123,"context_line":"            expected_url \u003d locations[0][\u0027url\u0027]"},{"line_number":124,"context_line":"        image.locations \u003d locations"},{"line_number":125,"context_line":"        store_utils.update_store_in_locations(context, image, image_repo)"},{"line_number":126,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_27c52a8f","line":123,"in_reply_to":"9f560f44_fbcf4c26","updated":"2020-09-04 20:48:21.000000000","message":"The migration is done at L#125 so if the case is valid then url will be like\ncinder://store-id/vol-id\nelse (in not valid case) will be\ncinder://vol-id","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d60f39b6dfa1ee312802c27e85079301cf4fa46","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            self.assertEqual(expected, image.locations[0][\u0027metadata\u0027].get("},{"line_number":130,"context_line":"                \u0027store\u0027))"},{"line_number":131,"context_line":"        else:"},{"line_number":132,"context_line":"            self.assertEqual(expected_url, image.locations[0].get(\u0027url\u0027))"},{"line_number":133,"context_line":"            self.assertEqual({}, image.locations[0][\u0027metadata\u0027])"},{"line_number":134,"context_line":"        self.assertEqual(save_call_count, image_repo.save.call_count)"},{"line_number":135,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_1b668056","line":132,"range":{"start_line":132,"start_character":12,"end_line":132,"end_character":73},"updated":"2020-09-04 19:43:42.000000000","message":"This is the same assertion as L128, so it should be outside the is_valid conditional right?","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac26c73405468c3125b550fe7160b4975d496b1a","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            self.assertEqual(expected, image.locations[0][\u0027metadata\u0027].get("},{"line_number":130,"context_line":"                \u0027store\u0027))"},{"line_number":131,"context_line":"        else:"},{"line_number":132,"context_line":"            self.assertEqual(expected_url, image.locations[0].get(\u0027url\u0027))"},{"line_number":133,"context_line":"            self.assertEqual({}, image.locations[0][\u0027metadata\u0027])"},{"line_number":134,"context_line":"        self.assertEqual(save_call_count, image_repo.save.call_count)"},{"line_number":135,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_47ec9eba","line":132,"range":{"start_line":132,"start_character":12,"end_line":132,"end_character":73},"in_reply_to":"9f560f44_1b668056","updated":"2020-09-04 20:48:21.000000000","message":"Will change this to be compatible with is_valid","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d60f39b6dfa1ee312802c27e85079301cf4fa46","unresolved":false,"context_lines":[{"line_number":130,"context_line":"                \u0027store\u0027))"},{"line_number":131,"context_line":"        else:"},{"line_number":132,"context_line":"            self.assertEqual(expected_url, image.locations[0].get(\u0027url\u0027))"},{"line_number":133,"context_line":"            self.assertEqual({}, image.locations[0][\u0027metadata\u0027])"},{"line_number":134,"context_line":"        self.assertEqual(save_call_count, image_repo.save.call_count)"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def test_update_cinder_store_location_valid_type(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_dbeb6875","line":133,"updated":"2020-09-04 19:43:42.000000000","message":"Can you add some more comments to this about what we\u0027re testing and why? I don\u0027t really understand why you\u0027re looking at the metadata here and why it\u0027s really different between the two cases, except that you\u0027re always passing in an empty dict in the test, which wouldn\u0027t be the case in real life, right?\n\nAlso, metadata, store_id and expected are always the same in both test cases, right? Why even make them parameters to this test helper? I think the metadata case should be different per above, but not the others, so why even have them?","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac26c73405468c3125b550fe7160b4975d496b1a","unresolved":false,"context_lines":[{"line_number":130,"context_line":"                \u0027store\u0027))"},{"line_number":131,"context_line":"        else:"},{"line_number":132,"context_line":"            self.assertEqual(expected_url, image.locations[0].get(\u0027url\u0027))"},{"line_number":133,"context_line":"            self.assertEqual({}, image.locations[0][\u0027metadata\u0027])"},{"line_number":134,"context_line":"        self.assertEqual(save_call_count, image_repo.save.call_count)"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def test_update_cinder_store_location_valid_type(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_e7233257","line":133,"in_reply_to":"9f560f44_dbeb6875","updated":"2020-09-04 20:48:21.000000000","message":"We call the glance store to check if the volume type of store is valid and update the url and metadata of image respectively.\nIf you see L#114, we are returning is_valid as True (if store is valid and update image) and False (if store is invalid don\u0027t update image) ... see [1] for reference\n\nSo if a store is valid, we update location and metada and check it that covers the first two assertions\nfor image_repo.save part, it is being called in is_valid\u003dTrue case with call count 1 which is default value at L#108 (as image gets updated) and not in in_valid\u003dFalse case hence call count 0 (as passed in L#144)\n\nI will update the test to show the same better\n\n[1] https://github.com/openstack/glance_store/blob/master/glance_store/_drivers/cinder.py#L441-L470","commit_id":"10ba8aeb27347bf76983f86bacc51ede30027ad9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a5a2519dc4b80fea7817f2a40f8421c290d8d0ad","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        store_utils.update_store_in_locations(context, image, image_repo)"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        if is_valid:"},{"line_number":123,"context_line":"            # This is the case when store is valid and location url,"},{"line_number":124,"context_line":"            # metadata are updated and image_repo.save is called"},{"line_number":125,"context_line":"            expected_url \u003d mock_url_prefix.return_value + \u0027/\u0027 + volume_id"},{"line_number":126,"context_line":"            self.assertEqual(expected_url, image.locations[0].get(\u0027url\u0027))"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_47f65e5f","line":123,"range":{"start_line":123,"start_character":45,"end_line":123,"end_character":50},"updated":"2020-09-04 20:59:47.000000000","message":"Valid doesn\u0027t really mean much to me here, or the is_valid flag. Just to be clear, this means \"is valid for migration\" right? This would make more sense to me if this was called \"is_legacy\" or \"needs_migration\".","commit_id":"df19b8cf75c7413a6090cc628b55160c4371694a"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"108183f422b76ed64019d4dc527373dd3f7875e3","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        store_utils.update_store_in_locations(context, image, image_repo)"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        if is_valid:"},{"line_number":123,"context_line":"            # This is the case when store is valid and location url,"},{"line_number":124,"context_line":"            # metadata are updated and image_repo.save is called"},{"line_number":125,"context_line":"            expected_url \u003d mock_url_prefix.return_value + \u0027/\u0027 + volume_id"},{"line_number":126,"context_line":"            self.assertEqual(expected_url, image.locations[0].get(\u0027url\u0027))"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_8617cfee","line":123,"range":{"start_line":123,"start_character":45,"end_line":123,"end_character":50},"in_reply_to":"9f560f44_2a3b2fd1","updated":"2020-09-07 07:02:47.000000000","message":"Done","commit_id":"df19b8cf75c7413a6090cc628b55160c4371694a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d685f0cc68a8d263e89c88fcdaf573fd27b6948","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        store_utils.update_store_in_locations(context, image, image_repo)"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        if is_valid:"},{"line_number":123,"context_line":"            # This is the case when store is valid and location url,"},{"line_number":124,"context_line":"            # metadata are updated and image_repo.save is called"},{"line_number":125,"context_line":"            expected_url \u003d mock_url_prefix.return_value + \u0027/\u0027 + volume_id"},{"line_number":126,"context_line":"            self.assertEqual(expected_url, image.locations[0].get(\u0027url\u0027))"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_2a3b2fd1","line":123,"range":{"start_line":123,"start_character":45,"end_line":123,"end_character":50},"in_reply_to":"9f560f44_47f65e5f","updated":"2020-09-04 21:19:53.000000000","message":"I see now why is_valid makes sense. I think more words here would really help make it clearer to a reader why you\u0027re doing this. For this case, something like:\n\n # This is the case where we found an image that has an\n # old-style URL which does not include the store name,\n # but for which we know the corresponding store that\n # refers to the volume type that backs it. We expect that\n # the URL should be updated to point to the store/volume from\n # just a naked pointer to the volume, as was the old\n # format.\n\nand then something like this for the bottom case:\n\n # Here, we\u0027ve got an image backed by a volume which does\n # not have a corresponding store specifying the volume_type.\n # Expect that we leave these alone and do not touch the\n # location URL since we cannot update it with a valid store.","commit_id":"df19b8cf75c7413a6090cc628b55160c4371694a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a5a2519dc4b80fea7817f2a40f8421c290d8d0ad","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            # This is the case when store is invalid and location url,"},{"line_number":132,"context_line":"            # metadata are not updated and image_repo.save is not called"},{"line_number":133,"context_line":"            self.assertEqual(locations[0][\u0027url\u0027],"},{"line_number":134,"context_line":"                             image.locations[0].get(\u0027url\u0027))"},{"line_number":135,"context_line":"            self.assertEqual({}, image.locations[0][\u0027metadata\u0027])"},{"line_number":136,"context_line":"            self.assertEqual(0, image_repo.save.call_count)"},{"line_number":137,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_478bfebe","line":134,"updated":"2020-09-04 20:59:47.000000000","message":"Moving this didn\u0027t really help anything for me. I understand you\u0027re checking that this is not changed, but you\u0027re still asserting that the url is something that it won\u0027t be in real life, right? Here, locations[0][\u0027url\u0027] is \"cinder://$volume_id\", but in the post-migration case, this would already be \"cinder://$store/$volume_id\" right?","commit_id":"df19b8cf75c7413a6090cc628b55160c4371694a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d685f0cc68a8d263e89c88fcdaf573fd27b6948","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            # This is the case when store is invalid and location url,"},{"line_number":132,"context_line":"            # metadata are not updated and image_repo.save is not called"},{"line_number":133,"context_line":"            self.assertEqual(locations[0][\u0027url\u0027],"},{"line_number":134,"context_line":"                             image.locations[0].get(\u0027url\u0027))"},{"line_number":135,"context_line":"            self.assertEqual({}, image.locations[0][\u0027metadata\u0027])"},{"line_number":136,"context_line":"            self.assertEqual(0, image_repo.save.call_count)"},{"line_number":137,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_aa273fb6","line":134,"in_reply_to":"9f560f44_478bfebe","updated":"2020-09-04 21:19:53.000000000","message":"Okay, I see now after talking on IRC, that this \"!is_valid\" case is for a situation where the config post-upgrade has changed such that there is no store configured that matches the volume type backing the image That\u0027s different from what I was interpreting this case to be, so it makes sense now why we\u0027re checking the url that we are.","commit_id":"df19b8cf75c7413a6090cc628b55160c4371694a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a5a2519dc4b80fea7817f2a40f8421c290d8d0ad","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                                                   \u0027fast-cinder\u0027)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def test_update_cinder_store_location_invalid_type(self):"},{"line_number":143,"context_line":"        self._test_update_cinder_store_in_location({}, \u0027fast-cinder\u0027,"},{"line_number":144,"context_line":"                                                   \u0027fast-cinder\u0027,"},{"line_number":145,"context_line":"                                                   is_valid\u003dFalse)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_e7b33299","line":144,"range":{"start_line":143,"start_character":51,"end_line":144,"end_character":65},"updated":"2020-09-04 20:59:47.000000000","message":"These are still exactly the same for both cases, so why pass them in?\n\nWouldn\u0027t it be more correct to pass in the expected value for metadata, so that update_store_in_locations() sees the value like it would be in real life, even though you expect it not to change?","commit_id":"df19b8cf75c7413a6090cc628b55160c4371694a"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"108183f422b76ed64019d4dc527373dd3f7875e3","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                                                   \u0027fast-cinder\u0027)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def test_update_cinder_store_location_invalid_type(self):"},{"line_number":143,"context_line":"        self._test_update_cinder_store_in_location({}, \u0027fast-cinder\u0027,"},{"line_number":144,"context_line":"                                                   \u0027fast-cinder\u0027,"},{"line_number":145,"context_line":"                                                   is_valid\u003dFalse)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_26e36307","line":144,"range":{"start_line":143,"start_character":51,"end_line":144,"end_character":65},"in_reply_to":"9f560f44_e7b33299","updated":"2020-09-07 07:02:47.000000000","message":"Done","commit_id":"df19b8cf75c7413a6090cc628b55160c4371694a"}]}