)]}'
{"trove/instance/models.py":[{"author":{"_account_id":36080,"name":"Erkin Mussurmankulov","display_name":"Eric","email":"erkin.mussurmankulov@ps.kz","username":"mongoose404","status":"PS Cloud services employee"},"change_message_id":"998956ec7f6f54a539b4d89bff65eab63cc8a49e","unresolved":true,"context_lines":[{"line_number":1674,"context_line":"        task_api.API(self.context).reboot(self.id)"},{"line_number":1675,"context_line":""},{"line_number":1676,"context_line":"    def restart(self):"},{"line_number":1677,"context_line":"        if self.db_info.server_status \u003d\u003d \"SHUTOFF\":"},{"line_number":1678,"context_line":"            self._validate_can_start_server()"},{"line_number":1679,"context_line":"            LOG.info(\"Compute server of instance %s is SHUTOFF, starting it.\","},{"line_number":1680,"context_line":"                     self.id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"0ae15402_6f8b8c19","line":1677,"updated":"2026-10-13 14:47:53.000000000","message":"Hello! Thank you for the patch. I can see a reason behind this logic, but this code breaks the single responsibility principle, so IMO the `restart` method shouldn\u0027t do `reboot` instead, even if the guest agent is down. It\u0027s a weird and undocumented behavior, which may lead to data loss in some edge cases (e.g., an incomplete upgrade process, an out-of-space case, or a combination of both). That\u0027s not what we want, right? )\n\nI propose allowing the reboot method to be called for OS users instead.\n\nAlso, I propose implementing this change in a separate MR for convenience and in order to match OS development principles.\n\nThus, if we allow the reboot method to users, we should implement the required changes in the `trove-pythonclient` repo too. \nIn the python-troveclient `reboot` method, we should warn users that this operation will take some time and may lead to unexpected results in some cases (but it should be safe for the database data) and ask for confirmation like `yes/no` or/and `type yes to continue` before doing the reboot itself. That will remove discrepancies and help users to differentiate `reboot` from `restart`. Names are very similar, and you should know what each of them does exactly.\nAlso, we should mention that there is a much safer and quicker `restart` method, which the user may want to run instead of a full Nova instance `reboot`. And we may also add a `-y`/`--yes` flag so the user will be able to force it without confirmation, if they like to.\n\nWhat do you think?\n\n__________\n\nFrom our real experience, allowing users to reboot the instance may be used several times. For example, under very high load, control-plane messages may time out, and the guest agent may become unresponsive. So instead of calling the cloud operator for help, which may take several hours, users may reboot the Nova instance themselves.\n*It\u0027s a strong argument for me to have this feature in the upstream.*\n\n----------\n\nThe rest of the patch LGTM, but the release note is missing. The patch introduces a big change in the trove behavior for users, and it should be documented.","commit_id":"1fee58d6e26589d62d73761d72273c3c5f3be04c"},{"author":{"_account_id":37632,"name":"Dmitriy Chubinidze","email":"dcu995@gmail.com","username":"chubinidzedr"},"change_message_id":"37883ce2d4afaffbdcc3837d6853b4a29b0ba846","unresolved":false,"context_lines":[{"line_number":1674,"context_line":"        task_api.API(self.context).reboot(self.id)"},{"line_number":1675,"context_line":""},{"line_number":1676,"context_line":"    def restart(self):"},{"line_number":1677,"context_line":"        if self.db_info.server_status \u003d\u003d \"SHUTOFF\":"},{"line_number":1678,"context_line":"            self._validate_can_start_server()"},{"line_number":1679,"context_line":"            LOG.info(\"Compute server of instance %s is SHUTOFF, starting it.\","},{"line_number":1680,"context_line":"                     self.id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"8e6488b7_2b652a8c","line":1677,"in_reply_to":"0ae15402_6f8b8c19","updated":"2026-10-14 23:50:00.000000000","message":"Hi, thanks for your detailed the review.\nI agree that restart shouldn’t silently turn into a Nova reboot, considering it didn\u0027t do it previously. \nI’ve removed that logic from this patch, which now only reports a SHUTOFF server as SHUTDOWN instead of UNKNOWN. Also added the release note with tests.\n\nI\u0027ll add user side nova instance reboot in next patch, with applying correct changes in trove client for rebooting as well.","commit_id":"1fee58d6e26589d62d73761d72273c3c5f3be04c"}]}