)]}'
{"nodepool/driver/statemachine.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"45c3851a2526ae1cef53db414a085758c1c92ea8","unresolved":true,"context_lines":[{"line_number":344,"context_line":"            except kze.NotEmptyError:"},{"line_number":345,"context_line":"                self.log.info(\"NotEmptyError in deleting Zk node \""},{"line_number":346,"context_line":"                              f\"id\u003d{node.external_id}, it seems the node \""},{"line_number":347,"context_line":"                              \"is being deleted by DeletedNodeWorker\")"},{"line_number":348,"context_line":"            self.manager.nodeDeletedNotification(node)"},{"line_number":349,"context_line":"        return True"},{"line_number":350,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"66cd1614_2f43e442","line":347,"updated":"2023-03-30 12:43:43.000000000","message":"A few things:\n1. I\u0027d not include the exception type and the term `DeletedNodeWorker` in the log message. Both sound a bit too \"technical\" to me.\n2. I think we could also up the log level to \"warning\"\n3. (nit) Logs should use %-formatting (`log.info(\"...%s ...\", variable`) instead of f-strings\n\nMore generally I\u0027m wondering if we can always be sure that the `NotEmptyError` is the result of a race between the SM and the worker trying to delete the same node. Another question would be if we want to execute `self.manager.nodeDeletedNotification(node)` in this case.","commit_id":"5df4ea718f127188c3476f6af3627e07ba2419ea"},{"author":{"_account_id":33134,"name":"Dong Zhang","email":"dong.zhang@bmw.de","username":"dongzhang"},"change_message_id":"bbc558083e74e87353b4508dfeecf4df76b1d709","unresolved":true,"context_lines":[{"line_number":344,"context_line":"            except kze.NotEmptyError:"},{"line_number":345,"context_line":"                self.log.info(\"NotEmptyError in deleting Zk node \""},{"line_number":346,"context_line":"                              f\"id\u003d{node.external_id}, it seems the node \""},{"line_number":347,"context_line":"                              \"is being deleted by DeletedNodeWorker\")"},{"line_number":348,"context_line":"            self.manager.nodeDeletedNotification(node)"},{"line_number":349,"context_line":"        return True"},{"line_number":350,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"e5ed6518_c996aa2d","line":347,"in_reply_to":"66cd1614_2f43e442","updated":"2023-03-30 12:52:38.000000000","message":"1. To be honest, I have a difficult time in thinking of a proper log message here, anyone has a better idea is welcome.\n2. \"warning\" sounds reasonable.\n3. I actually checked the log around that place, I see both formats are in use, so I just selected one.\n\nI checked the code, it seems at this place, the `NotEmptyError` should be only caused by the `DeleteNodeWorker`. And about the `elf.manager.nodeDeletedNotification(node)` I was also struggling a bit if it should be called when this exception happens. Any suggestion is welcome.","commit_id":"5df4ea718f127188c3476f6af3627e07ba2419ea"}]}