)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"034fbb44f4d31777f21385083dc0a5e5261ef7d0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b2d511d6_96a18284","updated":"2023-01-19 18:29:34.000000000","message":"See comment inline.","commit_id":"bef34a5422eb1e9fd27267586eb6b14d25441d09"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"238bc45e20849dd27771399c7e834efc201cb671","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"cf64e18d_6d053040","updated":"2023-10-23 21:32:03.000000000","message":"As per discussion in vPTG today, submissions to OpenStack must be made by humans. IMO, for this to merge, it must be resubmitted by a human who has signed the iCLA.\n\nI\u0027m unsure how it\u0027s possible for the iCLA to have been properly signed by an account that doesn\u0027t seem to represent a single individual.","commit_id":"0b1a91a78a16c413f046e14e7afda0048ec4f8ba"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ffc0b9d0dfebe660463f8f4c4077e2a9c89f712d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"605fd385_df28e25d","updated":"2023-07-21 15:15:40.000000000","message":"I think the change is OK.  Putting a hold on this until we hear back about whether the CLA is valid.  See discussion in #openstack-tc:\n\nhttps://meetings.opendev.org/irclogs/%23openstack-tc/%23openstack-tc.2023-07-21.log.html#t2023-07-21T14:41:0","commit_id":"0b1a91a78a16c413f046e14e7afda0048ec4f8ba"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"a28e6f54227c8613f895ff68dcc51a8c33fd7474","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5106d950_87b0e205","updated":"2023-02-01 14:38:56.000000000","message":"LGTM","commit_id":"0b1a91a78a16c413f046e14e7afda0048ec4f8ba"},{"author":{"_account_id":33609,"name":"XuQi","display_name":"Inori","email":"xuq.fnstxz@fujitsu.com","username":"inori"},"change_message_id":"b0bcbbea280cbf5c1321cc55a6dda305b195d21a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"dbe5eba7_d02b2407","updated":"2023-02-15 03:00:17.000000000","message":"LGTM","commit_id":"0b1a91a78a16c413f046e14e7afda0048ec4f8ba"},{"author":{"_account_id":34489,"name":"Matheus Andrade","email":"matheus.handrade15@gmail.com","username":"matheusandrade"},"change_message_id":"75332a198d3da80a2d9d4823ac153334852f3ebe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6f33dba2_c62e03d7","updated":"2023-07-21 14:40:36.000000000","message":"LGTM!","commit_id":"0b1a91a78a16c413f046e14e7afda0048ec4f8ba"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"f072148a46306efc26680baad45799e6de483ee4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"bf31b747_24b3554f","updated":"2023-07-21 14:24:28.000000000","message":"Looks good now","commit_id":"0b1a91a78a16c413f046e14e7afda0048ec4f8ba"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"f597b8da4662a6e14f621a3339b652b4c144d071","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"68d69159_bc231d7d","updated":"2023-07-21 13:57:40.000000000","message":"Looks good to me - thank you!","commit_id":"0b1a91a78a16c413f046e14e7afda0048ec4f8ba"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"a7d3a8113ee5fa7a905f9fc434aafeafd8468234","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"54db9494_86d9df85","updated":"2023-02-15 05:55:06.000000000","message":"lgtm","commit_id":"0b1a91a78a16c413f046e14e7afda0048ec4f8ba"}],"cinder/volume/api.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"034fbb44f4d31777f21385083dc0a5e5261ef7d0","unresolved":true,"context_lines":[{"line_number":210,"context_line":"    def _is_encrypted(self,"},{"line_number":211,"context_line":"                      volume_type: objects.volume_type.VolumeType) -\u003e bool:"},{"line_number":212,"context_line":"        specs \u003d volume_type.get(\u0027extra_specs\u0027, {})"},{"line_number":213,"context_line":"        return \u0027encryption\u0027 in specs"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def create(self,"},{"line_number":216,"context_line":"               context: context.RequestContext,"}],"source_content_type":"text/x-python","patch_set":1,"id":"e5efc747_48743f10","line":213,"updated":"2023-01-19 18:29:34.000000000","message":"I think you may have found a different bug.\n\nIt looks like the intention in the current code is to return False when \u0027encryption\u0027 is not in extra_specs or when \u0027encryption\u0027 is present but an empty dict.  If that\u0027s right, then your proposed replacement is incorrect.  But then so is the current code, because \u0027{} is not {}\u0027 is always True -- we should be using !\u003d instead.","commit_id":"bef34a5422eb1e9fd27267586eb6b14d25441d09"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ffc0b9d0dfebe660463f8f4c4077e2a9c89f712d","unresolved":false,"context_lines":[{"line_number":210,"context_line":"    def _is_encrypted(self,"},{"line_number":211,"context_line":"                      volume_type: objects.volume_type.VolumeType) -\u003e bool:"},{"line_number":212,"context_line":"        specs \u003d volume_type.get(\u0027extra_specs\u0027, {})"},{"line_number":213,"context_line":"        return \u0027encryption\u0027 in specs"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def create(self,"},{"line_number":216,"context_line":"               context: context.RequestContext,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7767032c_616769c7","line":213,"in_reply_to":"373cd252_88837774","updated":"2023-07-21 15:15:40.000000000","message":"Done","commit_id":"bef34a5422eb1e9fd27267586eb6b14d25441d09"},{"author":{"_account_id":35682,"name":"OpenRefactory Research","display_name":"OpenRefactory Research","email":"research@openrefactory.com","username":"openrefactory"},"change_message_id":"7c914b3a064c3d08b58266c62263ddf63aae6dcc","unresolved":true,"context_lines":[{"line_number":210,"context_line":"    def _is_encrypted(self,"},{"line_number":211,"context_line":"                      volume_type: objects.volume_type.VolumeType) -\u003e bool:"},{"line_number":212,"context_line":"        specs \u003d volume_type.get(\u0027extra_specs\u0027, {})"},{"line_number":213,"context_line":"        return \u0027encryption\u0027 in specs"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def create(self,"},{"line_number":216,"context_line":"               context: context.RequestContext,"}],"source_content_type":"text/x-python","patch_set":1,"id":"373cd252_88837774","line":213,"in_reply_to":"e5efc747_48743f10","updated":"2023-01-23 05:29:37.000000000","message":"I have changed the logic. Please Review.","commit_id":"bef34a5422eb1e9fd27267586eb6b14d25441d09"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ba9ac37694369df62497d83bf265aa3187cba7e9","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        specs \u003d volume_type.get(\u0027extra_specs\u0027, {})"},{"line_number":213,"context_line":"        if \u0027encryption\u0027 not in specs:"},{"line_number":214,"context_line":"            return False"},{"line_number":215,"context_line":"        return specs.get(\u0027encryption\u0027, {}) !\u003d {}"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def create(self,"},{"line_number":218,"context_line":"               context: context.RequestContext,"}],"source_content_type":"text/x-python","patch_set":2,"id":"42f4daf9_dd49935e","line":215,"range":{"start_line":215,"start_character":15,"end_line":215,"end_character":48},"updated":"2023-10-24 00:35:06.000000000","message":"is this even correct\n\nshoudl this not be \n\n    def _is_encrypted(self,\n                      volume_type: objects.volume_type.VolumeType) -\u003e bool:\n        specs \u003d volume_type.get(\u0027extra_specs\u0027, {})\n        return bool(specs.get(\u0027encryption\u0027, False))\n        \nchanging form \"is not {}\" to \"!\u003d {}\"\n\nis changing form an identity check to a equaltiy check.\n\nbool({}) is false bool(\u003cany non empty dict\u003e) is true\n\n\nin the previous code we check if the key is in the dict with\n\n        if \u0027encryption\u0027 not in specs:\n            return False\nso as a result \"specs.get(\u0027encryption\u0027, {})\" will never use the decault value and it will only return {} if the value of specs[\u0027encyption\u0027] is {}\n\nits corerct that we should not be doing an identity check her but the submited code while techinally corect is not how this should be fix. \nthe issue is not just the use of identiy instead of equality for non monostate types like None the code is also doing extra work in the form of a contaiens check then a lookup of the key if it exists.\n\nif we didnt care about line lenght this could be one lien\n    \n    def _is_encrypted(self,\n                      volume_type: objects.volume_type.VolumeType) -\u003e bool:\n        return bool(volume_type.get(\u0027extra_specs\u0027, {}).get(\u0027encryption\u0027,False))\n        \nthe icla/liciens conserns are one thing but from a code quality point of view there  is no explainign of why the change is correct in the commit, there is no bug or code comment and no test coverage to assert the previosuly incorect behavior or that the new behavior is correct.\n\nfor any of those reaosns this should have a -1\nbeign submitted by a bot is just another checkmark in the -1 category.","commit_id":"0b1a91a78a16c413f046e14e7afda0048ec4f8ba"}]}