)]}'
{"/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":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":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"}],"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":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"}],"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":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":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"}]}