)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a0e061957ae0b5a7ebe5ea4eca475b3a0c6f3763","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add support volume local cache"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Use volume type extra spec \u0027cachable\u0027 to identify if a volume can be"},{"line_number":10,"context_line":"cached or not. If it is set to \u0027\u003cis\u003e True\u0027, then it means the volume can"},{"line_number":11,"context_line":"be cached in compute node locally via caching software (e.g. OpenCAS)"},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3fa7e38b_c4855355","line":9,"range":{"start_line":9,"start_character":28,"end_line":9,"end_character":36},"updated":"2020-02-17 19:41:28.000000000","message":"The cinder-spec lists this as \"cacheable\" which I think is a more common spelling, so it should be used here.","commit_id":"127b57e6b6e348576ca8f54067211ef45ccd5284"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"d81df409ac00f15e86a1b71c3076855910ff6d51","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add support volume local cache"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Use volume type extra spec \u0027cachable\u0027 to identify if a volume can be"},{"line_number":10,"context_line":"cached or not. If it is set to \u0027\u003cis\u003e True\u0027, then it means the volume can"},{"line_number":11,"context_line":"be cached in compute node locally via caching software (e.g. OpenCAS)"},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7f423b7e_d86815bc","line":9,"range":{"start_line":9,"start_character":28,"end_line":9,"end_character":36},"in_reply_to":"3fa7e38b_c4855355","updated":"2020-04-11 10:14:43.000000000","message":"Done","commit_id":"127b57e6b6e348576ca8f54067211ef45ccd5284"}],"cinder/api/contrib/types_extra_specs.py":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"1a88d851b30c2ff56d27157ad09d3d1e6bb52fde","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            image_service_store_id \u003d specs[\u0027image_service:store_id\u0027]"},{"line_number":77,"context_line":"            image_utils.validate_stores_id(context, image_service_store_id)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        # Check if multiattach be set with cacheable"},{"line_number":80,"context_line":"        multiattach \u003d volume_types.get_volume_type_extra_specs("},{"line_number":81,"context_line":"            type_id, key\u003d\u0027multiattach\u0027)"},{"line_number":82,"context_line":"        cacheable \u003d volume_types.get_volume_type_extra_specs("}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_a96258d6","line":79,"updated":"2020-06-25 21:21:31.000000000","message":"I don\u0027t see test cases associated with this check.","commit_id":"5823b2ba6e5736d8def336e4e05c63a37947b805"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"e3c008ec993356c03cacea82ee98e19f0e476bb3","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            image_service_store_id \u003d specs[\u0027image_service:store_id\u0027]"},{"line_number":77,"context_line":"            image_utils.validate_stores_id(context, image_service_store_id)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        # Check if multiattach be set with cacheable"},{"line_number":80,"context_line":"        multiattach \u003d volume_types.get_volume_type_extra_specs("},{"line_number":81,"context_line":"            type_id, key\u003d\u0027multiattach\u0027)"},{"line_number":82,"context_line":"        cacheable \u003d volume_types.get_volume_type_extra_specs("}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_90f07669","line":79,"in_reply_to":"bf51134e_62c323ee","updated":"2020-07-13 09:04:59.000000000","message":"Good catch, thanks Brian.","commit_id":"5823b2ba6e5736d8def336e4e05c63a37947b805"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0f98a8edfc7cc21be47101e592584097dd5f6427","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            image_service_store_id \u003d specs[\u0027image_service:store_id\u0027]"},{"line_number":77,"context_line":"            image_utils.validate_stores_id(context, image_service_store_id)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        # Check if multiattach be set with cacheable"},{"line_number":80,"context_line":"        multiattach \u003d volume_types.get_volume_type_extra_specs("},{"line_number":81,"context_line":"            type_id, key\u003d\u0027multiattach\u0027)"},{"line_number":82,"context_line":"        cacheable \u003d volume_types.get_volume_type_extra_specs("}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_62c323ee","line":79,"in_reply_to":"bf51134e_a96258d6","updated":"2020-06-27 23:02:03.000000000","message":"Definitely need unit tests.  I\u0027m not sure the logic at line 85 will catch the situation where the volume type currently has neither multiattach nor cacheable set, which will make \u0027multiattach\u0027 False and \u0027cacheable\u0027 False at lines 80 and 82, respectively.  That will make the condition at line 85 fail even if the incoming specs are trying to set both multiattach and cacheable at the same time.","commit_id":"5823b2ba6e5736d8def336e4e05c63a37947b805"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"e3c008ec993356c03cacea82ee98e19f0e476bb3","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            image_service_store_id \u003d specs[\u0027image_service:store_id\u0027]"},{"line_number":77,"context_line":"            image_utils.validate_stores_id(context, image_service_store_id)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        # Check if multiattach be set with cacheable"},{"line_number":80,"context_line":"        multiattach \u003d volume_types.get_volume_type_extra_specs("},{"line_number":81,"context_line":"            type_id, key\u003d\u0027multiattach\u0027)"},{"line_number":82,"context_line":"        cacheable \u003d volume_types.get_volume_type_extra_specs("}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_30decad9","line":79,"in_reply_to":"bf51134e_a96258d6","updated":"2020-07-13 09:04:59.000000000","message":"Done, thanks","commit_id":"5823b2ba6e5736d8def336e4e05c63a37947b805"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0f98a8edfc7cc21be47101e592584097dd5f6427","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        if \u0027image_service:store_id\u0027 in body:"},{"line_number":115,"context_line":"            image_service_store_id \u003d body[\u0027image_service:store_id\u0027]"},{"line_number":116,"context_line":"            image_utils.validate_stores_id(context, image_service_store_id)"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        db.volume_type_extra_specs_update_or_create(context,"},{"line_number":119,"context_line":"                                                    type_id,"},{"line_number":120,"context_line":"                                                    body)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_0262270a","line":117,"updated":"2020-06-27 23:02:03.000000000","message":"Don\u0027t we also need to check here?","commit_id":"5823b2ba6e5736d8def336e4e05c63a37947b805"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"e3c008ec993356c03cacea82ee98e19f0e476bb3","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        if \u0027image_service:store_id\u0027 in body:"},{"line_number":115,"context_line":"            image_service_store_id \u003d body[\u0027image_service:store_id\u0027]"},{"line_number":116,"context_line":"            image_utils.validate_stores_id(context, image_service_store_id)"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        db.volume_type_extra_specs_update_or_create(context,"},{"line_number":119,"context_line":"                                                    type_id,"},{"line_number":120,"context_line":"                                                    body)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_d0e66eb0","line":117,"in_reply_to":"bf51134e_0262270a","updated":"2020-07-13 09:04:59.000000000","message":"added, thanks","commit_id":"5823b2ba6e5736d8def336e4e05c63a37947b805"}],"cinder/tests/unit/api/contrib/test_types_extra_specs.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5e8e42534b1de924181bf6cde4f0aecf60e9a090","unresolved":false,"context_lines":[{"line_number":458,"context_line":"    @mock.patch(\u0027cinder.volume.volume_types.get_volume_type_extra_specs\u0027)"},{"line_number":459,"context_line":"    def test_volume_type_specs_get(self, get_extra_specs):"},{"line_number":460,"context_line":"        ret_multiattach \u003d \u0027\u003cis\u003e True\u0027"},{"line_number":461,"context_line":"        ret_cacheable \u003d \u0027\u003cis\u003e False\u0027"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"        def side_get_specs(type_id, key):"},{"line_number":464,"context_line":"            if key \u003d\u003d \u0027multiattach\u0027:"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf51134e_9743c5af","line":461,"range":{"start_line":461,"start_character":24,"end_line":461,"end_character":36},"updated":"2020-07-21 20:47:53.000000000","message":"If this could be in extra-specs, you should probably add some tests to make sure it\u0027s handled correctly.  (I know your code is fine, but someone might refactor and just convert the value to a boolean without seeing what\u0027s in there.)","commit_id":"a3d4e092b8b0700fdec1dca73d2e0e7d1b3efb56"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"ed4c26dfbc604c1b1f3421edd9baf88ddab72bea","unresolved":false,"context_lines":[{"line_number":458,"context_line":"    @mock.patch(\u0027cinder.volume.volume_types.get_volume_type_extra_specs\u0027)"},{"line_number":459,"context_line":"    def test_volume_type_specs_get(self, get_extra_specs):"},{"line_number":460,"context_line":"        ret_multiattach \u003d \u0027\u003cis\u003e True\u0027"},{"line_number":461,"context_line":"        ret_cacheable \u003d \u0027\u003cis\u003e False\u0027"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"        def side_get_specs(type_id, key):"},{"line_number":464,"context_line":"            if key \u003d\u003d \u0027multiattach\u0027:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_117c7381","line":461,"range":{"start_line":461,"start_character":24,"end_line":461,"end_character":36},"in_reply_to":"bf51134e_9743c5af","updated":"2020-08-05 10:33:11.000000000","message":"thanks, all set to \u0027\u0027 as you suggested in below comment.\n\nOnly \u0027\u003cis\u003e True\u0027 is True, other strings or unset will be treated as False","commit_id":"a3d4e092b8b0700fdec1dca73d2e0e7d1b3efb56"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5e8e42534b1de924181bf6cde4f0aecf60e9a090","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        get_extra_specs.side_effect \u003d side_get_specs"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"        specs \u003d {\u0027multiattach\u0027: \u0027\u003cis\u003e True\u0027,"},{"line_number":472,"context_line":"                 \u0027cacheable\u0027: \u0027\u003cis\u003e True\u0027}"},{"line_number":473,"context_line":"        self.assertRaises(webob.exc.HTTPBadRequest,"},{"line_number":474,"context_line":"                          self.controller._check_cacheable,"},{"line_number":475,"context_line":"                          specs, \u0027typeid\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf51134e_37fe396b","line":472,"updated":"2020-07-21 20:47:53.000000000","message":"I think you should set ret_multiattach \u003d \u0027\u0027 and ret_cacheable \u003d \u0027\u0027 before the first test, so we can test the case where neither one is already in extra-specs, but both are in the request.","commit_id":"a3d4e092b8b0700fdec1dca73d2e0e7d1b3efb56"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"ed4c26dfbc604c1b1f3421edd9baf88ddab72bea","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        get_extra_specs.side_effect \u003d side_get_specs"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"        specs \u003d {\u0027multiattach\u0027: \u0027\u003cis\u003e True\u0027,"},{"line_number":472,"context_line":"                 \u0027cacheable\u0027: \u0027\u003cis\u003e True\u0027}"},{"line_number":473,"context_line":"        self.assertRaises(webob.exc.HTTPBadRequest,"},{"line_number":474,"context_line":"                          self.controller._check_cacheable,"},{"line_number":475,"context_line":"                          specs, \u0027typeid\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_51726bae","line":472,"in_reply_to":"bf51134e_37fe396b","updated":"2020-08-05 10:33:11.000000000","message":"done, thanks","commit_id":"a3d4e092b8b0700fdec1dca73d2e0e7d1b3efb56"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0ba8316ac44a7f406995b43841a21adfb06637f9","unresolved":false,"context_lines":[{"line_number":456,"context_line":"                          body\u003dbody)"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"    @mock.patch(\u0027cinder.volume.volume_types.get_volume_type_extra_specs\u0027)"},{"line_number":459,"context_line":"    def test_volume_type_specs_get(self, get_extra_specs):"},{"line_number":460,"context_line":"        ret_multiattach \u003d \u0027\u0027"},{"line_number":461,"context_line":"        ret_cacheable \u003d \u0027\u0027"},{"line_number":462,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_0ae750e6","line":459,"updated":"2020-08-20 02:39:44.000000000","message":"this test should probably be named test__check_cacheable so it\u0027s clear what you\u0027re testing here.","commit_id":"1498a3fda812bc843ce8f5fcd299c2426720ea17"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"be7452ccfb9d031dfbd820008352ba5c47ae6bc0","unresolved":false,"context_lines":[{"line_number":456,"context_line":"                          body\u003dbody)"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"    @mock.patch(\u0027cinder.volume.volume_types.get_volume_type_extra_specs\u0027)"},{"line_number":459,"context_line":"    def test_volume_type_specs_get(self, get_extra_specs):"},{"line_number":460,"context_line":"        ret_multiattach \u003d \u0027\u0027"},{"line_number":461,"context_line":"        ret_cacheable \u003d \u0027\u0027"},{"line_number":462,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_aa1a9f79","line":459,"in_reply_to":"9f560f44_0ae750e6","updated":"2020-08-20 10:19:26.000000000","message":"make sense, done.","commit_id":"1498a3fda812bc843ce8f5fcd299c2426720ea17"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0ba8316ac44a7f406995b43841a21adfb06637f9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"9f560f44_eab49cd3","line":490,"updated":"2020-08-20 02:39:44.000000000","message":"I think it would be worth adding a few more checks to make sure that \u0027\u003cis\u003e False\u0027 behaves the way you\u0027d expect.  (From looking at the code, I\u0027m sure it will, but might as well add the checks to prevent regressions.)","commit_id":"1498a3fda812bc843ce8f5fcd299c2426720ea17"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"be7452ccfb9d031dfbd820008352ba5c47ae6bc0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"9f560f44_ca15534b","line":490,"in_reply_to":"9f560f44_eab49cd3","updated":"2020-08-20 10:19:26.000000000","message":"agree, done, thanks","commit_id":"1498a3fda812bc843ce8f5fcd299c2426720ea17"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"1a46f8966fb6d6b6a8e5360e43b6a53e7aa30dd4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"9f560f44_f6375efd","line":502,"updated":"2020-08-20 14:01:05.000000000","message":"Thanks for adding those, but what I\u0027m really interested in seeing is what happens when:\n\n1 - ret_multiattach \u003d \u0027\u003cis\u003e False\u0027 and specs \u003d {\u0027multiattach\u0027: \u0027\u003cis\u003e True\u0027} (should NOT get an exception)\n\n2 - ret_multiattach \u003d \u0027\u003cis\u003e True\u0027 and specs \u003d {\u0027multiattach\u0027: \u0027\u003cis\u003e False\u0027, \u0027cacheable\u0027: \u0027\u003cis\u003e True\u0027} (should fail)\n\n3 - ret_multiattach \u003d \u0027\u003cis\u003e False\u0027 and specs \u003d {\u0027multiattach\u0027: \u0027\u003cis\u003e False\u0027, \u0027cacheable\u0027: \u0027\u003cis\u003e True\u0027} (should be OK)\n\nI can see someone actually doing 2 \u0026 3 (that is, they want to make sure that they\u0027re not setting both at the same time, so they explicitly say one is False in the request).","commit_id":"29b873ba75153ae373c42e8064de25db5b17d04c"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"5f1d3a0823a776713c702073426f11736e43acf9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"9f560f44_3f96bd3f","line":502,"in_reply_to":"9f560f44_f6375efd","updated":"2020-08-21 03:33:32.000000000","message":"thanks Brian, I have reworked, hope I have understood your point correctly.","commit_id":"29b873ba75153ae373c42e8064de25db5b17d04c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"36e8681ba0bc29e24e6e25309f73fa36d56b7132","unresolved":false,"context_lines":[{"line_number":497,"context_line":"        ret_multiattach \u003d \u0027\u003cis\u003e True\u0027"},{"line_number":498,"context_line":"        ret_cacheable \u003d \u0027\u0027"},{"line_number":499,"context_line":"        specs \u003d {\u0027multiattach\u0027: \u0027\u003cis\u003e False\u0027, \u0027cacheable\u0027: \u0027\u003cis\u003e True\u0027}"},{"line_number":500,"context_line":"        # Should NOT setting both at the same time"},{"line_number":501,"context_line":"        self.assertRaises(webob.exc.HTTPBadRequest,"},{"line_number":502,"context_line":"                          self.controller._check_cacheable,"},{"line_number":503,"context_line":"                          specs, \u0027typeid\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_b29bfb14","line":500,"updated":"2020-08-24 16:44:12.000000000","message":"Yes, this shows that multiattach\u003d\u003dFalse in the input doesn\u0027t mask the fact that multiattach\u003d\u003dTrue is already set in the DB.","commit_id":"13f69fc8093114860ac0b151aafa5fd5708cac96"}],"cinder/volume/driver.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"705d13d79a25d7fa7fc20c35db0fce1d7e9f46ce","unresolved":false,"context_lines":[{"line_number":1467,"context_line":"        :param volume: The volume to be attached"},{"line_number":1468,"context_line":"        :param connector: Dictionary containing information about what is being"},{"line_number":1469,"context_line":"                          connected to."},{"line_number":1470,"context_line":"        :returns conn_info: A dictionary of connection information."},{"line_number":1471,"context_line":"        \"\"\""},{"line_number":1472,"context_line":"        return"},{"line_number":1473,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_9bd72b24","line":1470,"updated":"2020-09-05 13:38:42.000000000","message":"Add something like:\n\n..note::\n  Whether or not a volume is \u0027cacheable\u0027 for a volume local cache on the\n  hypervisor is normally configured in the volume-type extra-specs.\n  Support may be disabled at the driver level, however, by returning\n  \"cacheable\": False in the conn_info.  This will override any setting\n  in the volume-type extra-specs.","commit_id":"13f69fc8093114860ac0b151aafa5fd5708cac96"}],"cinder/volume/drivers/lvm.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a0e061957ae0b5a7ebe5ea4eca475b3a0c6f3763","unresolved":false,"context_lines":[{"line_number":267,"context_line":"            filter_function\u003dself.get_filter_function(),"},{"line_number":268,"context_line":"            goodness_function\u003dself.get_goodness_function(),"},{"line_number":269,"context_line":"            multiattach\u003dTrue,"},{"line_number":270,"context_line":"            cachable\u003dTrue,"},{"line_number":271,"context_line":"            backend_state\u003d\u0027up\u0027"},{"line_number":272,"context_line":"        ))"},{"line_number":273,"context_line":"        data[\"pools\"].append(single_pool)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_64e01f2f","line":270,"range":{"start_line":270,"start_character":12,"end_line":270,"end_character":25},"updated":"2020-02-17 19:41:28.000000000","message":"I don\u0027t think this needs to be in the LVM driver.","commit_id":"127b57e6b6e348576ca8f54067211ef45ccd5284"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"d81df409ac00f15e86a1b71c3076855910ff6d51","unresolved":false,"context_lines":[{"line_number":267,"context_line":"            filter_function\u003dself.get_filter_function(),"},{"line_number":268,"context_line":"            goodness_function\u003dself.get_goodness_function(),"},{"line_number":269,"context_line":"            multiattach\u003dTrue,"},{"line_number":270,"context_line":"            cachable\u003dTrue,"},{"line_number":271,"context_line":"            backend_state\u003d\u0027up\u0027"},{"line_number":272,"context_line":"        ))"},{"line_number":273,"context_line":"        data[\"pools\"].append(single_pool)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f423b7e_d8965595","line":270,"range":{"start_line":270,"start_character":12,"end_line":270,"end_character":25},"in_reply_to":"3fa7e38b_64e01f2f","updated":"2020-04-11 10:14:43.000000000","message":"volume type extra-spec without prefix \"capabilities:\" will be treated as filter. See: https://github.com/openstack/cinder/blob/master/cinder/scheduler/filters/capabilities_filter.py#L62\n\nSo extra-spec \"cacheable\" will be treated as filter. As discussed in spec, some backends like nfs and rbd will not be supported. So I\u0027m trying to enable the supported backends explicitly. Any backends not marked \"cacheable\u003dTrue\" will not be supported by default. It is like white list mode. We may can go through backends drivers and add more drivers as supported later in a following patch.","commit_id":"127b57e6b6e348576ca8f54067211ef45ccd5284"}],"cinder/volume/manager.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a0e061957ae0b5a7ebe5ea4eca475b3a0c6f3763","unresolved":false,"context_lines":[{"line_number":1758,"context_line":""},{"line_number":1759,"context_line":"        # Add cachable flag to connection_info if not set in the driver."},{"line_number":1760,"context_line":"        if typeid:"},{"line_number":1761,"context_line":"            if conn_info[\u0027data\u0027].get(\u0027cachable\u0027) is None:"},{"line_number":1762,"context_line":"                cachable \u003d volume_types.get_volume_type_extra_specs("},{"line_number":1763,"context_line":"                    typeid, key\u003d\u0027cachable\u0027)"},{"line_number":1764,"context_line":"                conn_info[\u0027data\u0027][\u0027cachable\u0027] \u003d cachable"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_84e51b40","line":1761,"range":{"start_line":1761,"start_character":38,"end_line":1761,"end_character":46},"updated":"2020-02-17 19:41:28.000000000","message":"\"cacheable\"","commit_id":"127b57e6b6e348576ca8f54067211ef45ccd5284"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"d81df409ac00f15e86a1b71c3076855910ff6d51","unresolved":false,"context_lines":[{"line_number":1758,"context_line":""},{"line_number":1759,"context_line":"        # Add cachable flag to connection_info if not set in the driver."},{"line_number":1760,"context_line":"        if typeid:"},{"line_number":1761,"context_line":"            if conn_info[\u0027data\u0027].get(\u0027cachable\u0027) is None:"},{"line_number":1762,"context_line":"                cachable \u003d volume_types.get_volume_type_extra_specs("},{"line_number":1763,"context_line":"                    typeid, key\u003d\u0027cachable\u0027)"},{"line_number":1764,"context_line":"                conn_info[\u0027data\u0027][\u0027cachable\u0027] \u003d cachable"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f423b7e_b86549e4","line":1761,"range":{"start_line":1761,"start_character":38,"end_line":1761,"end_character":46},"in_reply_to":"3fa7e38b_84e51b40","updated":"2020-04-11 10:14:43.000000000","message":"Done","commit_id":"127b57e6b6e348576ca8f54067211ef45ccd5284"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5e8e42534b1de924181bf6cde4f0aecf60e9a090","unresolved":false,"context_lines":[{"line_number":1765,"context_line":"            if conn_info[\u0027data\u0027].get(\u0027cacheable\u0027) is None:"},{"line_number":1766,"context_line":"                cacheable \u003d volume_types.get_volume_type_extra_specs("},{"line_number":1767,"context_line":"                    typeid, key\u003d\u0027cacheable\u0027)"},{"line_number":1768,"context_line":"                conn_info[\u0027data\u0027][\u0027cacheable\u0027] \u003d cacheable"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"        # Add discard flag to connection_info if not set in the driver and"},{"line_number":1771,"context_line":"        # configured to be reported."}],"source_content_type":"text/x-python","patch_set":5,"id":"bf51134e_57c64d4b","line":1768,"range":{"start_line":1768,"start_character":15,"end_line":1768,"end_character":46},"updated":"2020-07-21 20:47:53.000000000","message":"do you want a boolean here?  (compare to line 2627, and also line 77 in test_attachments_manager.py)  I think this is a string if the key is set.\n\nIf so, you got lucky in test_attachment_update.  You may want to add a special-purpose test in test_attachments_manager.py to check to make sure you get \u0027cacheable\u0027: True in the connection_info (or maybe there\u0027s a connection info test somewhere you can adapt).","commit_id":"a3d4e092b8b0700fdec1dca73d2e0e7d1b3efb56"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"ed4c26dfbc604c1b1f3421edd9baf88ddab72bea","unresolved":false,"context_lines":[{"line_number":1765,"context_line":"            if conn_info[\u0027data\u0027].get(\u0027cacheable\u0027) is None:"},{"line_number":1766,"context_line":"                cacheable \u003d volume_types.get_volume_type_extra_specs("},{"line_number":1767,"context_line":"                    typeid, key\u003d\u0027cacheable\u0027)"},{"line_number":1768,"context_line":"                conn_info[\u0027data\u0027][\u0027cacheable\u0027] \u003d cacheable"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"        # Add discard flag to connection_info if not set in the driver and"},{"line_number":1771,"context_line":"        # configured to be reported."}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_563d41db","line":1768,"range":{"start_line":1768,"start_character":15,"end_line":1768,"end_character":46},"in_reply_to":"bf51134e_57c64d4b","updated":"2020-08-05 10:33:11.000000000","message":"Yes, you are correct, should be boolean here. I use this value in https://review.opendev.org/#/c/663549/6/os_brick/caches/__init__.py@62, and in my tests I set to \u0027\u003cis\u003e True\u0027 or unset, so did not find this potential issue(any string will be treated as True, even it\u0027s not \u0027\u003cis\u003e True\u0027), thanks Brian","commit_id":"a3d4e092b8b0700fdec1dca73d2e0e7d1b3efb56"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0ba8316ac44a7f406995b43841a21adfb06637f9","unresolved":false,"context_lines":[{"line_number":1765,"context_line":"            if conn_info[\u0027data\u0027].get(\u0027cacheable\u0027) is None:"},{"line_number":1766,"context_line":"                cacheable \u003d volume_types.get_volume_type_extra_specs("},{"line_number":1767,"context_line":"                    typeid, key\u003d\u0027cacheable\u0027)"},{"line_number":1768,"context_line":"                conn_info[\u0027data\u0027][\u0027cacheable\u0027] \u003d (True"},{"line_number":1769,"context_line":"                                                  if cacheable \u003d\u003d \u0027\u003cis\u003e True\u0027"},{"line_number":1770,"context_line":"                                                  else False)"},{"line_number":1771,"context_line":""},{"line_number":1772,"context_line":"        # Add discard flag to connection_info if not set in the driver and"},{"line_number":1773,"context_line":"        # configured to be reported."}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_aa62a4ab","line":1770,"range":{"start_line":1768,"start_character":49,"end_line":1770,"end_character":61},"updated":"2020-08-20 02:39:44.000000000","message":"I think you could simplify this to:\n\n  cacheable \u003d\u003d \u0027\u003cis\u003e True\u0027","commit_id":"1498a3fda812bc843ce8f5fcd299c2426720ea17"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"be7452ccfb9d031dfbd820008352ba5c47ae6bc0","unresolved":false,"context_lines":[{"line_number":1765,"context_line":"            if conn_info[\u0027data\u0027].get(\u0027cacheable\u0027) is None:"},{"line_number":1766,"context_line":"                cacheable \u003d volume_types.get_volume_type_extra_specs("},{"line_number":1767,"context_line":"                    typeid, key\u003d\u0027cacheable\u0027)"},{"line_number":1768,"context_line":"                conn_info[\u0027data\u0027][\u0027cacheable\u0027] \u003d (True"},{"line_number":1769,"context_line":"                                                  if cacheable \u003d\u003d \u0027\u003cis\u003e True\u0027"},{"line_number":1770,"context_line":"                                                  else False)"},{"line_number":1771,"context_line":""},{"line_number":1772,"context_line":"        # Add discard flag to connection_info if not set in the driver and"},{"line_number":1773,"context_line":"        # configured to be reported."}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_6a2b6794","line":1770,"range":{"start_line":1768,"start_character":49,"end_line":1770,"end_character":61},"in_reply_to":"9f560f44_aa62a4ab","updated":"2020-08-20 10:19:26.000000000","message":"yes, yes :(","commit_id":"1498a3fda812bc843ce8f5fcd299c2426720ea17"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"f86e89846d1830df9b418f0843b4e758994d7577","unresolved":false,"context_lines":[{"line_number":1765,"context_line":"            if conn_info[\u0027data\u0027].get(\u0027cacheable\u0027) is None:"},{"line_number":1766,"context_line":"                cacheable \u003d volume_types.get_volume_type_extra_specs("},{"line_number":1767,"context_line":"                    typeid, key\u003d\u0027cacheable\u0027)"},{"line_number":1768,"context_line":"                conn_info[\u0027data\u0027][\u0027cacheable\u0027] \u003d (cacheable \u003d\u003d \u0027\u003cis\u003e True\u0027)"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"        # Add discard flag to connection_info if not set in the driver and"},{"line_number":1771,"context_line":"        # configured to be reported."}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_f95d9c79","line":1768,"updated":"2020-09-04 13:50:54.000000000","message":"This forces the cacheable flag on for every driver in cinder.\nI think we should leave this up to the driver, just like we do in the case of setting the multiattach flag.","commit_id":"13f69fc8093114860ac0b151aafa5fd5708cac96"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"705d13d79a25d7fa7fc20c35db0fce1d7e9f46ce","unresolved":false,"context_lines":[{"line_number":1765,"context_line":"            if conn_info[\u0027data\u0027].get(\u0027cacheable\u0027) is None:"},{"line_number":1766,"context_line":"                cacheable \u003d volume_types.get_volume_type_extra_specs("},{"line_number":1767,"context_line":"                    typeid, key\u003d\u0027cacheable\u0027)"},{"line_number":1768,"context_line":"                conn_info[\u0027data\u0027][\u0027cacheable\u0027] \u003d (cacheable \u003d\u003d \u0027\u003cis\u003e True\u0027)"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"        # Add discard flag to connection_info if not set in the driver and"},{"line_number":1771,"context_line":"        # configured to be reported."}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_fba627c0","line":1768,"in_reply_to":"9f560f44_b99684ee","updated":"2020-09-05 13:38:42.000000000","message":"See my comment in driver.py about how you can address Walt\u0027s point.\n\nWalt brings up an interesting point, but I am worried about the opposite situation, namely, what if the driver sets cacheable \u003d True?  While it may be safe for a driver to turn off caching all the time, we cannot allow it to turn caching on all the time, because that\u0027s supposed to be controlled by the operator via the extra-specs.  So I think you need to add a check here for when the conn_info[\u0027data\u0027][\u0027cacheable\u0027] is defined.  Something like this:\n\n  if typeid:\n      cacheable \u003d volume_types.get_volume_type_extra_specs(\n          typeid, key\u003d\u0027cacheable\u0027)\n      if conn_info[\u0027data\u0027].get(\u0027cacheable\u0027) is not None:\n          driver_setting \u003d bool(conn_info[\u0027data\u0027][\u0027cacheable\u0027])\n          # override a True driver_setting but respect False\n          conn_info[\u0027data\u0027][\u0027cacheable\u0027] \u003d (driver_setting and\n                                            (cacheable \u003d\u003d \u0027\u003cis\u003e True\u0027))\n      else:\n          conn_info[\u0027data\u0027][\u0027cacheable\u0027] \u003d (cacheable \u003d\u003d \u0027\u003cis\u003e True\u0027)\n\nThere\u0027s probably a better way to do it, but you can figure that out when you write the tests :).","commit_id":"13f69fc8093114860ac0b151aafa5fd5708cac96"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"5f933a15520540cb99035205e8c016a646719c6c","unresolved":false,"context_lines":[{"line_number":1765,"context_line":"            if conn_info[\u0027data\u0027].get(\u0027cacheable\u0027) is None:"},{"line_number":1766,"context_line":"                cacheable \u003d volume_types.get_volume_type_extra_specs("},{"line_number":1767,"context_line":"                    typeid, key\u003d\u0027cacheable\u0027)"},{"line_number":1768,"context_line":"                conn_info[\u0027data\u0027][\u0027cacheable\u0027] \u003d (cacheable \u003d\u003d \u0027\u003cis\u003e True\u0027)"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"        # Add discard flag to connection_info if not set in the driver and"},{"line_number":1771,"context_line":"        # configured to be reported."}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_b99684ee","line":1768,"in_reply_to":"9f560f44_f95d9c79","updated":"2020-09-04 14:00:23.000000000","message":"I think it\u0027s safer to rely on the driver to set this flag.","commit_id":"13f69fc8093114860ac0b151aafa5fd5708cac96"}]}