)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"9980211cff7685961115871c23a367e428a386ba","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"da558bfd_c8f3f14a","updated":"2026-08-28 00:41:02.000000000","message":"-1 is for the following:\n\n1. I think you need some more negative tests around the QoS values.  I don\u0027t think you need to check every possible combination necessarily, but maybe exercise each value at least once in a test.  Recommend using DDT for this.\n\n2. There are two places where I think an exception should be used instead of returning None based on context from the rest of the code.\n\n3. Potential inconsistent usage of floating-point values (could be release-noted away, I suppose).\n\n4. Need release notes.","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"e840030b36e2425e1bb01141aa2a0afbe7591201","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c4c32904_9ce10a81","updated":"2026-08-28 00:43:06.000000000","message":"Add one more note about needing documentation.","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38274,"name":"Jyotsna Lothe","display_name":"Jyotsna Lothe","email":"jyotsna.lothe@hpe.com","username":"jlothe"},"change_message_id":"13cf5a3dac8396ec18e572c90ef7585177dfe9b9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e8134968_d5c7a334","updated":"2026-08-18 12:41:01.000000000","message":"PR is ready for review","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38274,"name":"Jyotsna Lothe","display_name":"Jyotsna Lothe","email":"jyotsna.lothe@hpe.com","username":"jlothe"},"change_message_id":"128552d3652cab458d674145263274cbbf69977b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"bcbc8942_08adc439","in_reply_to":"2d7bc969_d7f6e0cb","updated":"2026-08-31 12:31:59.000000000","message":"Addressed feedback from agalica: \n1) Added negative test cases using DDT as suggested.\n2) Added exception handling at both the places instead of returning None\n3) Added handling for float values","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"e840030b36e2425e1bb01141aa2a0afbe7591201","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2d7bc969_d7f6e0cb","in_reply_to":"6ee76653_540b84ba","updated":"2026-08-28 00:43:06.000000000","message":"Also need documentation.","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38274,"name":"Jyotsna Lothe","display_name":"Jyotsna Lothe","email":"jyotsna.lothe@hpe.com","username":"jlothe"},"change_message_id":"82ac7496a6bfcc47f83497375d6672c3428ac897","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"20dc072a_9869c7ec","in_reply_to":"c4c32904_9ce10a81","updated":"2026-08-31 12:57:34.000000000","message":"Added release notes in updated patch","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"7539a3448a01737d3b2c71c036e65971222c20d9","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6ee76653_540b84ba","in_reply_to":"da558bfd_c8f3f14a","updated":"2026-08-28 00:42:17.000000000","message":"Ahh, done in by (broken?) list comment formatting:\n\nI think you need some more negative tests around the QoS values. I don\u0027t think you need to check every possible combination necessarily, but maybe exercise each value at least once in a test. Recommend using DDT for this.\n\nThere are two places where I think an exception should be used instead of returning None based on context from the rest of the code.\n\nPotential inconsistent usage of floating-point values (could be release-noted away, I suppose).\n\nNeed release notes.","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38274,"name":"Jyotsna Lothe","display_name":"Jyotsna Lothe","email":"jyotsna.lothe@hpe.com","username":"jlothe"},"change_message_id":"128552d3652cab458d674145263274cbbf69977b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e34c23ea_0b2928e1","updated":"2026-08-31 12:31:59.000000000","message":"@anthony.galica@hitachivantara.com thanks for feedback provided. We have addressed all the review comments and updated patch.","commit_id":"0a38e83fe6e5500fdf30c2312195a5688a963a7e"},{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"833dfd1fd1a7d741ea7ad31b418b1d4a00487ee1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a7a40cb6_fd255328","updated":"2026-09-09 14:54:52.000000000","message":"@jyotsna.lothe@hpe.com need to check zuul","commit_id":"0a38e83fe6e5500fdf30c2312195a5688a963a7e"},{"author":{"_account_id":38274,"name":"Jyotsna Lothe","display_name":"Jyotsna Lothe","email":"jyotsna.lothe@hpe.com","username":"jlothe"},"change_message_id":"82ac7496a6bfcc47f83497375d6672c3428ac897","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f3d797cd_7dfd2a54","updated":"2026-08-31 12:57:34.000000000","message":"Added release notes in updated patch","commit_id":"0a38e83fe6e5500fdf30c2312195a5688a963a7e"},{"author":{"_account_id":38274,"name":"Jyotsna Lothe","display_name":"Jyotsna Lothe","email":"jyotsna.lothe@hpe.com","username":"jlothe"},"change_message_id":"c7ad8f25d785b20a2b83aaad0bb224ac9640f935","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9e3d6e02_091d44c8","updated":"2026-10-05 11:37:31.000000000","message":"I also request core reviewers to take a look. If we get early feedback we can address it quickly and plan for merge as soon as Indri opens for merges.","commit_id":"0a38e83fe6e5500fdf30c2312195a5688a963a7e"},{"author":{"_account_id":38274,"name":"Jyotsna Lothe","display_name":"Jyotsna Lothe","email":"jyotsna.lothe@hpe.com","username":"jlothe"},"change_message_id":"eb9a09a4c804f5cd75fb4050b358db09d8da505a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f31421a2_67f9e28b","updated":"2026-09-01 09:48:03.000000000","message":"recheck","commit_id":"0a38e83fe6e5500fdf30c2312195a5688a963a7e"},{"author":{"_account_id":38274,"name":"Jyotsna Lothe","display_name":"Jyotsna Lothe","email":"jyotsna.lothe@hpe.com","username":"jlothe"},"change_message_id":"bbe56347dbdf50b3ff268a6a6de9f36b2a096aa9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b361d296_32abaa49","updated":"2026-09-13 05:56:51.000000000","message":"recheck tempest-integrated-storage","commit_id":"0a38e83fe6e5500fdf30c2312195a5688a963a7e"}],"cinder/tests/unit/volume/drivers/hpe/test_alletramp.py":[{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"9980211cff7685961115871c23a367e428a386ba","unresolved":true,"context_lines":[{"line_number":674,"context_line":"                              \u0027vvs-capacity\u0027)"},{"line_number":675,"context_line":"            mock_create_qos.assert_not_called()"},{"line_number":676,"context_line":""},{"line_number":677,"context_line":"    def test_set_qos_rule_capacity_qos_rejects_legacy_limits(self):"},{"line_number":678,"context_line":"        qos \u003d {\u0027total_iops_sec_per_gb\u0027: \u0027100\u0027, \u0027maxIOPS\u0027: \u00274000\u0027}"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"        with mock.patch.object(self.service, \u0027_is_alletra_mp\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"794aeda6_25f27678","line":677,"updated":"2026-08-28 00:41:02.000000000","message":"Can we get DDT tests here to test more combinations?","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38274,"name":"Jyotsna Lothe","display_name":"Jyotsna Lothe","email":"jyotsna.lothe@hpe.com","username":"jlothe"},"change_message_id":"128552d3652cab458d674145263274cbbf69977b","unresolved":false,"context_lines":[{"line_number":674,"context_line":"                              \u0027vvs-capacity\u0027)"},{"line_number":675,"context_line":"            mock_create_qos.assert_not_called()"},{"line_number":676,"context_line":""},{"line_number":677,"context_line":"    def test_set_qos_rule_capacity_qos_rejects_legacy_limits(self):"},{"line_number":678,"context_line":"        qos \u003d {\u0027total_iops_sec_per_gb\u0027: \u0027100\u0027, \u0027maxIOPS\u0027: \u00274000\u0027}"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"        with mock.patch.object(self.service, \u0027_is_alletra_mp\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"b3711c5a_1b48e58a","line":677,"in_reply_to":"794aeda6_25f27678","updated":"2026-08-31 12:31:59.000000000","message":"Done","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"9980211cff7685961115871c23a367e428a386ba","unresolved":true,"context_lines":[{"line_number":689,"context_line":"                              \u0027vvs-capacity\u0027)"},{"line_number":690,"context_line":"            mock_create_qos.assert_not_called()"},{"line_number":691,"context_line":""},{"line_number":692,"context_line":"    def test_set_qos_rule_capacity_qos_rejects_mixed_directions(self):"},{"line_number":693,"context_line":"        qos \u003d {\u0027read_iops_sec_per_gb\u0027: \u0027100\u0027,"},{"line_number":694,"context_line":"               \u0027write_iops_sec_per_gb\u0027: \u0027100\u0027}"},{"line_number":695,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"2d6e5310_6030fdbc","line":692,"updated":"2026-08-28 00:41:02.000000000","message":"Can we get DDT tests here to test more combinations?","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38274,"name":"Jyotsna Lothe","display_name":"Jyotsna Lothe","email":"jyotsna.lothe@hpe.com","username":"jlothe"},"change_message_id":"128552d3652cab458d674145263274cbbf69977b","unresolved":false,"context_lines":[{"line_number":689,"context_line":"                              \u0027vvs-capacity\u0027)"},{"line_number":690,"context_line":"            mock_create_qos.assert_not_called()"},{"line_number":691,"context_line":""},{"line_number":692,"context_line":"    def test_set_qos_rule_capacity_qos_rejects_mixed_directions(self):"},{"line_number":693,"context_line":"        qos \u003d {\u0027read_iops_sec_per_gb\u0027: \u0027100\u0027,"},{"line_number":694,"context_line":"               \u0027write_iops_sec_per_gb\u0027: \u0027100\u0027}"},{"line_number":695,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"e740636a_07df1df6","line":692,"in_reply_to":"2d6e5310_6030fdbc","updated":"2026-08-31 12:31:59.000000000","message":"Marked as resolved.","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"}],"cinder/volume/drivers/hpe/alletramp_service.py":[{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"9980211cff7685961115871c23a367e428a386ba","unresolved":true,"context_lines":[{"line_number":907,"context_line":""},{"line_number":908,"context_line":"    def _get_valid_qos_int(self, key, value):"},{"line_number":909,"context_line":"        if not self._is_qos_value_set(value):"},{"line_number":910,"context_line":"            return None"},{"line_number":911,"context_line":""},{"line_number":912,"context_line":"        try:"},{"line_number":913,"context_line":"            int_value \u003d int(value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"d7256a17_9f833a6c","line":910,"updated":"2026-08-28 00:41:02.000000000","message":"Should this throw an exception?  It looks like the return value can get used directly as an integer in other places in the code (which will trigger an exception whose source may be harder to debug).","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38274,"name":"Jyotsna Lothe","display_name":"Jyotsna Lothe","email":"jyotsna.lothe@hpe.com","username":"jlothe"},"change_message_id":"128552d3652cab458d674145263274cbbf69977b","unresolved":false,"context_lines":[{"line_number":907,"context_line":""},{"line_number":908,"context_line":"    def _get_valid_qos_int(self, key, value):"},{"line_number":909,"context_line":"        if not self._is_qos_value_set(value):"},{"line_number":910,"context_line":"            return None"},{"line_number":911,"context_line":""},{"line_number":912,"context_line":"        try:"},{"line_number":913,"context_line":"            int_value \u003d int(value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"8cca6cbf_e8129f07","line":910,"in_reply_to":"d7256a17_9f833a6c","updated":"2026-08-31 12:31:59.000000000","message":"Marked as resolved. Added exception handling instead of returning none.","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"9980211cff7685961115871c23a367e428a386ba","unresolved":true,"context_lines":[{"line_number":926,"context_line":""},{"line_number":927,"context_line":"    def _get_valid_qos_float(self, key, value):"},{"line_number":928,"context_line":"        if not self._is_qos_value_set(value):"},{"line_number":929,"context_line":"            return None"},{"line_number":930,"context_line":""},{"line_number":931,"context_line":"        try:"},{"line_number":932,"context_line":"            float_value \u003d float(value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ef36af7_a8e4a73b","line":929,"updated":"2026-08-28 00:41:02.000000000","message":"Should this throw an exception?  It looks like the return value can get used directly as an integer in other places in the code (which will trigger an exception whose source may be harder to debug).","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38274,"name":"Jyotsna Lothe","display_name":"Jyotsna Lothe","email":"jyotsna.lothe@hpe.com","username":"jlothe"},"change_message_id":"128552d3652cab458d674145263274cbbf69977b","unresolved":false,"context_lines":[{"line_number":926,"context_line":""},{"line_number":927,"context_line":"    def _get_valid_qos_float(self, key, value):"},{"line_number":928,"context_line":"        if not self._is_qos_value_set(value):"},{"line_number":929,"context_line":"            return None"},{"line_number":930,"context_line":""},{"line_number":931,"context_line":"        try:"},{"line_number":932,"context_line":"            float_value \u003d float(value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c914824c_046e13f1","line":929,"in_reply_to":"3ef36af7_a8e4a73b","updated":"2026-08-31 12:31:59.000000000","message":"Marked as resolved.","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"9980211cff7685961115871c23a367e428a386ba","unresolved":true,"context_lines":[{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":"        latency \u003d self._get_valid_qos_float(\u0027latency\u0027, latency)"},{"line_number":1106,"context_line":"        if latency \u003e\u003d 1:"},{"line_number":1107,"context_line":"            qosRule[\u0027latencyGoal\u0027] \u003d int(latency)"},{"line_number":1108,"context_line":"        else:"},{"line_number":1109,"context_line":"            qosRule[\u0027latencyGoaluSecs\u0027] \u003d int(latency * 1000)"},{"line_number":1110,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"a1961c20_598c9466","line":1107,"updated":"2026-08-28 00:41:02.000000000","message":"I\u0027m wondering if uSecs should always be used.  What if someone sets a value of 1.5?","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"},{"author":{"_account_id":38274,"name":"Jyotsna Lothe","display_name":"Jyotsna Lothe","email":"jyotsna.lothe@hpe.com","username":"jlothe"},"change_message_id":"128552d3652cab458d674145263274cbbf69977b","unresolved":false,"context_lines":[{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":"        latency \u003d self._get_valid_qos_float(\u0027latency\u0027, latency)"},{"line_number":1106,"context_line":"        if latency \u003e\u003d 1:"},{"line_number":1107,"context_line":"            qosRule[\u0027latencyGoal\u0027] \u003d int(latency)"},{"line_number":1108,"context_line":"        else:"},{"line_number":1109,"context_line":"            qosRule[\u0027latencyGoaluSecs\u0027] \u003d int(latency * 1000)"},{"line_number":1110,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"70f2940f_d81c1f35","line":1107,"in_reply_to":"a1961c20_598c9466","updated":"2026-08-31 12:31:59.000000000","message":"Addressed the handling for float values like 1.5. Also added respective UT.","commit_id":"3ecb5e77e4817210f03d57977cfd26e1d2b75398"}]}
