)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"22c3545e2a4f4f6b5d963d5ebab70ec150ead303","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"79b65b0e_4e462577","updated":"2022-03-18 14:52:52.000000000","message":"See Luigi\u0027s comment in the code for a change worth making.  I have a suggestion inline for an improvement, but don\u0027t insist on it for this patch.  It could be done in a followup if you decide it\u0027s worth doing.","commit_id":"b1c08f626f2dc33e68e4a9068c3984060134a8ea"}],"cinder/tests/unit/volume/drivers/test_spdk.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"22c3545e2a4f4f6b5d963d5ebab70ec150ead303","unresolved":true,"context_lines":[{"line_number":511,"context_line":"        self.configuration.spdk_rpc_ip \u003d \"127.0.0.1\""},{"line_number":512,"context_line":"        self.configuration.spdk_rpc_port \u003d 8000"},{"line_number":513,"context_line":"        self.configuration.spdk_rpc_protocol \u003d \"https\""},{"line_number":514,"context_line":"        mock_safe_get \u003d mock.Mock()"},{"line_number":515,"context_line":"        mock_safe_get.return_value \u003d \u0027spdk-nvmeof\u0027"},{"line_number":516,"context_line":"        self.configuration.safe_get \u003d mock_safe_get"},{"line_number":517,"context_line":"        self.jsonrpcclient \u003d JSONRPCClient()"},{"line_number":518,"context_line":"        self.driver \u003d spdk_driver.SPDKDriver(configuration\u003d"}],"source_content_type":"text/x-python","patch_set":8,"id":"871afe70_e0e2e6ba","line":515,"range":{"start_line":514,"start_character":7,"end_line":515,"end_character":50},"updated":"2022-03-18 14:52:52.000000000","message":"Since you inherit from cinder.tests.unit.test.TestCase, you can use the \"flags\" to modify configuration in tests, and then you wouldn\u0027t have to mock safe_get.\n\nYou\u0027d just call\n\n  self.flags(\u0027volume_backend_name\u0027, \u0027whatever\u0027)\n\nat the beginning of the test function, and then \u0027whatever\u0027 should show up in the driver stats.  This would make your test more readable, too.  Looking at the test, it\u0027s not obvious where \u0027spdk-nvmeof\u0027 is coming from.","commit_id":"b1c08f626f2dc33e68e4a9068c3984060134a8ea"}],"cinder/volume/drivers/spdk.py":[{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"4dc29450b072bb2d60a0263c2d5fa147ba843dad","unresolved":true,"context_lines":[{"line_number":88,"context_line":"        \"\"\"Retrieve stats info from volume group.\"\"\""},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        LOG.debug(\u0027SPDK Updating volume stats\u0027)"},{"line_number":91,"context_line":"        status \u003d {\u0027volume_backend_name\u0027: \u0027SPDK\u0027,"},{"line_number":92,"context_line":"                  \u0027vendor_name\u0027: \u0027Open Source\u0027,"},{"line_number":93,"context_line":"                  \u0027driver_version\u0027: self.VERSION,"},{"line_number":94,"context_line":"                  \u0027storage_protocol\u0027: \u0027NVMe-oF\u0027}"}],"source_content_type":"text/x-python","patch_set":2,"id":"8b8837b2_15c4e296","line":91,"range":{"start_line":91,"start_character":41,"end_line":91,"end_character":47},"updated":"2022-03-02 01:15:14.000000000","message":"This should be also change to: self.configuration.safe_get(\"volume_backend_name\")\n\nand this should be better：self.configuration.safe_get(\"volume_backend_name\") or “SPDK”？","commit_id":"24d4f80202c5dd0105e2d02fef344156cba79271"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"976506271c1027cd7f0a9aa1e8cefafb691db35f","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        \"\"\"Retrieve stats info from volume group.\"\"\""},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        LOG.debug(\u0027SPDK Updating volume stats\u0027)"},{"line_number":91,"context_line":"        status \u003d {\u0027volume_backend_name\u0027: \u0027SPDK\u0027,"},{"line_number":92,"context_line":"                  \u0027vendor_name\u0027: \u0027Open Source\u0027,"},{"line_number":93,"context_line":"                  \u0027driver_version\u0027: self.VERSION,"},{"line_number":94,"context_line":"                  \u0027storage_protocol\u0027: \u0027NVMe-oF\u0027}"}],"source_content_type":"text/x-python","patch_set":2,"id":"893ad65b_3fce6bf5","line":91,"range":{"start_line":91,"start_character":41,"end_line":91,"end_character":47},"in_reply_to":"8b8837b2_15c4e296","updated":"2022-03-08 05:00:49.000000000","message":"fixed","commit_id":"24d4f80202c5dd0105e2d02fef344156cba79271"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"4dc29450b072bb2d60a0263c2d5fa147ba843dad","unresolved":true,"context_lines":[{"line_number":106,"context_line":"                total_size \u003d (lvs[\u0027total_data_clusters\u0027]"},{"line_number":107,"context_line":"                              * lvs[\u0027cluster_size\u0027]"},{"line_number":108,"context_line":"                              / units.Gi)"},{"line_number":109,"context_line":"                pool[\"volume_backend_name\"] \u003d ("},{"line_number":110,"context_line":"                    self.configuration.safe_get(\"volume_backend_name\"))"},{"line_number":111,"context_line":"                pool[\"vendor_name\"] \u003d \u0027Open Source\u0027"},{"line_number":112,"context_line":"                pool[\"driver_version\"] \u003d self.VERSION"},{"line_number":113,"context_line":"                pool[\"storage_protocol\"] \u003d \u0027NVMe-oF\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"6e674b34_f2efc1d8","line":110,"range":{"start_line":109,"start_character":46,"end_line":110,"end_character":71},"updated":"2022-03-02 01:15:14.000000000","message":"this should be better：self.configuration.safe_get(\"volume_backend_name\") or “SPDK”？","commit_id":"24d4f80202c5dd0105e2d02fef344156cba79271"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"976506271c1027cd7f0a9aa1e8cefafb691db35f","unresolved":false,"context_lines":[{"line_number":106,"context_line":"                total_size \u003d (lvs[\u0027total_data_clusters\u0027]"},{"line_number":107,"context_line":"                              * lvs[\u0027cluster_size\u0027]"},{"line_number":108,"context_line":"                              / units.Gi)"},{"line_number":109,"context_line":"                pool[\"volume_backend_name\"] \u003d ("},{"line_number":110,"context_line":"                    self.configuration.safe_get(\"volume_backend_name\"))"},{"line_number":111,"context_line":"                pool[\"vendor_name\"] \u003d \u0027Open Source\u0027"},{"line_number":112,"context_line":"                pool[\"driver_version\"] \u003d self.VERSION"},{"line_number":113,"context_line":"                pool[\"storage_protocol\"] \u003d \u0027NVMe-oF\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"8263b41b_95a6fa5b","line":110,"range":{"start_line":109,"start_character":46,"end_line":110,"end_character":71},"in_reply_to":"6e674b34_f2efc1d8","updated":"2022-03-08 05:00:49.000000000","message":"Done","commit_id":"24d4f80202c5dd0105e2d02fef344156cba79271"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"d3ff46de5ff5c65b8fa434d8ef0f209745b0df8f","unresolved":true,"context_lines":[{"line_number":88,"context_line":"        \"\"\"Retrieve stats info from volume group.\"\"\""},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        LOG.debug(\u0027SPDK Updating volume stats\u0027)"},{"line_number":91,"context_line":"        backend_name \u003d self.configuration.safe_get(\"volume_backend_name\")"},{"line_number":92,"context_line":"        status \u003d {\u0027volume_backend_name\u0027: backend_name or \u0027SPDK\u0027,"},{"line_number":93,"context_line":"                  \u0027vendor_name\u0027: \u0027Open Source\u0027,"},{"line_number":94,"context_line":"                  \u0027driver_version\u0027: self.VERSION,"}],"source_content_type":"text/x-python","patch_set":8,"id":"be48c66d_598b9a35","line":91,"updated":"2022-03-18 14:15:45.000000000","message":"Unfortunately Configuration.safe_get doesn\u0027t have a parameter to set the \"default\" if nothing else is set. This would be useful here.\n\nBut I\u0027d say in this case it would make sense to set backend_name to the right value, so that when it is used later (lines 92 and 110) there is not need to add the fallback logic. In other words, backend_name should contains the expected backend name when it is used.\nA simple if not backend_name: backend_name \u003d \u0027SPDK\u0027 or other equivalent syntax should do.","commit_id":"b1c08f626f2dc33e68e4a9068c3984060134a8ea"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"22c3545e2a4f4f6b5d963d5ebab70ec150ead303","unresolved":true,"context_lines":[{"line_number":88,"context_line":"        \"\"\"Retrieve stats info from volume group.\"\"\""},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        LOG.debug(\u0027SPDK Updating volume stats\u0027)"},{"line_number":91,"context_line":"        backend_name \u003d self.configuration.safe_get(\"volume_backend_name\")"},{"line_number":92,"context_line":"        status \u003d {\u0027volume_backend_name\u0027: backend_name or \u0027SPDK\u0027,"},{"line_number":93,"context_line":"                  \u0027vendor_name\u0027: \u0027Open Source\u0027,"},{"line_number":94,"context_line":"                  \u0027driver_version\u0027: self.VERSION,"}],"source_content_type":"text/x-python","patch_set":8,"id":"f18a4f89_b7423623","line":91,"in_reply_to":"be48c66d_598b9a35","updated":"2022-03-18 14:52:52.000000000","message":"-1: I agree with Luigi, it\u0027s better to make sure that backend_name has a suitable value here, and then you don\u0027t need to worry about someone leaving off the \"or \u0027SPDK\u0027\" part elsewhere in the code.","commit_id":"b1c08f626f2dc33e68e4a9068c3984060134a8ea"}]}