)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3d963ef6fdb629b7adabb46e42174ab86f194cae","unresolved":false,"context_lines":[{"line_number":4986,"context_line":"        \"\"\"Adds or removes the COMPUTE_STATUS_DISABLED trait for this host."},{"line_number":4987,"context_line":""},{"line_number":4988,"context_line":"        For each ComputeNode managed by this service, adds or removes the"},{"line_number":4989,"context_line":"        COMPUTE_STATUS_DISABLED traits to/from the associated resource provider"},{"line_number":4990,"context_line":"        in Placement."},{"line_number":4991,"context_line":""},{"line_number":4992,"context_line":"        :param context: nova auth RequestContext"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_18f26771","line":4989,"range":{"start_line":4989,"start_character":32,"end_line":4989,"end_character":38},"updated":"2019-07-12 23:50:17.000000000","message":"trait","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"342c1e2200f60f82d4a3033b8a9f4283f2ec67c9","unresolved":false,"context_lines":[{"line_number":5011,"context_line":"            except (exception.ResourceProviderTraitRetrievalFailed,"},{"line_number":5012,"context_line":"                    exception.ResourceProviderUpdateConflict,"},{"line_number":5013,"context_line":"                    exception.ResourceProviderUpdateFailed,"},{"line_number":5014,"context_line":"                    exception.TraitRetrievalFailed) as e:"},{"line_number":5015,"context_line":"                # This is best effort so just log a warning and continue. The"},{"line_number":5016,"context_line":"                # update_available_resource periodic task will sync the trait."},{"line_number":5017,"context_line":"                LOG.warning(\u0027An error occurred while updating \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_bcbe2bd2","line":5014,"updated":"2019-07-03 10:48:49.000000000","message":"I think you missed keystoneauth1.exceptions.ClientException. Or is it intentionally left for the catch all except Exception block?","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63c8e126268e6b959336e54d89a5c0ee2feee2b2","unresolved":false,"context_lines":[{"line_number":5011,"context_line":"            except (exception.ResourceProviderTraitRetrievalFailed,"},{"line_number":5012,"context_line":"                    exception.ResourceProviderUpdateConflict,"},{"line_number":5013,"context_line":"                    exception.ResourceProviderUpdateFailed,"},{"line_number":5014,"context_line":"                    exception.TraitRetrievalFailed) as e:"},{"line_number":5015,"context_line":"                # This is best effort so just log a warning and continue. The"},{"line_number":5016,"context_line":"                # update_available_resource periodic task will sync the trait."},{"line_number":5017,"context_line":"                LOG.warning(\u0027An error occurred while updating \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_fccee681","line":5014,"in_reply_to":"9fb8cfa7_333b46c9","updated":"2019-07-04 09:07:15.000000000","message":"works for me.","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3294f00c0b6f94d6989c774486e121256b48b9a","unresolved":false,"context_lines":[{"line_number":5011,"context_line":"            except (exception.ResourceProviderTraitRetrievalFailed,"},{"line_number":5012,"context_line":"                    exception.ResourceProviderUpdateConflict,"},{"line_number":5013,"context_line":"                    exception.ResourceProviderUpdateFailed,"},{"line_number":5014,"context_line":"                    exception.TraitRetrievalFailed) as e:"},{"line_number":5015,"context_line":"                # This is best effort so just log a warning and continue. The"},{"line_number":5016,"context_line":"                # update_available_resource periodic task will sync the trait."},{"line_number":5017,"context_line":"                LOG.warning(\u0027An error occurred while updating \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_333b46c9","line":5014,"in_reply_to":"9fb8cfa7_bcbe2bd2","updated":"2019-07-03 14:24:49.000000000","message":"Hmm, I\u0027m not sure if that was intentional but it\u0027s probably better to be handled as an unexpected exception and traced since if we can\u0027t communicate with placement we\u0027ve got bigger problems.","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"342c1e2200f60f82d4a3033b8a9f4283f2ec67c9","unresolved":false,"context_lines":[{"line_number":5017,"context_line":"                LOG.warning(\u0027An error occurred while updating \u0027"},{"line_number":5018,"context_line":"                            \u0027COMPUTE_STATUS_DISABLED trait on compute node \u0027"},{"line_number":5019,"context_line":"                            \u0027resource provider %s. Error: %s\u0027,"},{"line_number":5020,"context_line":"                            node.uuid, e.format_message())"},{"line_number":5021,"context_line":"            except Exception:"},{"line_number":5022,"context_line":"                LOG.exception(\u0027An error occurred while updating \u0027"},{"line_number":5023,"context_line":"                              \u0027COMPUTE_STATUS_DISABLED trait on compute node \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_1ccdf785","line":5020,"updated":"2019-07-03 10:48:49.000000000","message":"Do we want to mention in the log that the update_available_resource periodic task is expected to re-sync? In an env that such a task is set to low frequency this information would help troubleshooting computes that are left disable by the above error.","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63c8e126268e6b959336e54d89a5c0ee2feee2b2","unresolved":false,"context_lines":[{"line_number":5017,"context_line":"                LOG.warning(\u0027An error occurred while updating \u0027"},{"line_number":5018,"context_line":"                            \u0027COMPUTE_STATUS_DISABLED trait on compute node \u0027"},{"line_number":5019,"context_line":"                            \u0027resource provider %s. Error: %s\u0027,"},{"line_number":5020,"context_line":"                            node.uuid, e.format_message())"},{"line_number":5021,"context_line":"            except Exception:"},{"line_number":5022,"context_line":"                LOG.exception(\u0027An error occurred while updating \u0027"},{"line_number":5023,"context_line":"                              \u0027COMPUTE_STATUS_DISABLED trait on compute node \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_7ca0b6a6","line":5020,"in_reply_to":"7faddb67_af7cb9b5","updated":"2019-07-04 09:07:15.000000000","message":"Thanks. Fup works for me.","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"edd3b5805914cafd3474366f004dd505fd50f3ef","unresolved":false,"context_lines":[{"line_number":5017,"context_line":"                LOG.warning(\u0027An error occurred while updating \u0027"},{"line_number":5018,"context_line":"                            \u0027COMPUTE_STATUS_DISABLED trait on compute node \u0027"},{"line_number":5019,"context_line":"                            \u0027resource provider %s. Error: %s\u0027,"},{"line_number":5020,"context_line":"                            node.uuid, e.format_message())"},{"line_number":5021,"context_line":"            except Exception:"},{"line_number":5022,"context_line":"                LOG.exception(\u0027An error occurred while updating \u0027"},{"line_number":5023,"context_line":"                              \u0027COMPUTE_STATUS_DISABLED trait on compute node \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_af7cb9b5","line":5020,"in_reply_to":"7faddb67_ef1dd10f","updated":"2019-07-03 19:58:24.000000000","message":"Done: https://review.opendev.org/#/c/668986/","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3294f00c0b6f94d6989c774486e121256b48b9a","unresolved":false,"context_lines":[{"line_number":5017,"context_line":"                LOG.warning(\u0027An error occurred while updating \u0027"},{"line_number":5018,"context_line":"                            \u0027COMPUTE_STATUS_DISABLED trait on compute node \u0027"},{"line_number":5019,"context_line":"                            \u0027resource provider %s. Error: %s\u0027,"},{"line_number":5020,"context_line":"                            node.uuid, e.format_message())"},{"line_number":5021,"context_line":"            except Exception:"},{"line_number":5022,"context_line":"                LOG.exception(\u0027An error occurred while updating \u0027"},{"line_number":5023,"context_line":"                              \u0027COMPUTE_STATUS_DISABLED trait on compute node \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_3364a6e5","line":5020,"in_reply_to":"9fb8cfa7_1ccdf785","updated":"2019-07-03 14:24:49.000000000","message":"Yeah I could do that, and that might also be good information to put in the docs as well. If it\u0027s alright with you I\u0027ll throw that into a follow up patch while I\u0027m collecting comments.","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a4e0f9e2047034d51f2ed90f0aeee46e935ba081","unresolved":false,"context_lines":[{"line_number":5017,"context_line":"                LOG.warning(\u0027An error occurred while updating \u0027"},{"line_number":5018,"context_line":"                            \u0027COMPUTE_STATUS_DISABLED trait on compute node \u0027"},{"line_number":5019,"context_line":"                            \u0027resource provider %s. Error: %s\u0027,"},{"line_number":5020,"context_line":"                            node.uuid, e.format_message())"},{"line_number":5021,"context_line":"            except Exception:"},{"line_number":5022,"context_line":"                LOG.exception(\u0027An error occurred while updating \u0027"},{"line_number":5023,"context_line":"                              \u0027COMPUTE_STATUS_DISABLED trait on compute node \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_ef1dd10f","line":5020,"in_reply_to":"9fb8cfa7_3364a6e5","updated":"2019-07-03 19:31:40.000000000","message":"\u003e and that might also be good information to put in the docs as well.\n\nI guess something related to this is already in the docs:\n\nhttps://review.opendev.org/#/c/668752/1/doc/source/admin/configuration/schedulers.rst@93\n\nBut it\u0027s not talking about the error case, so I\u0027ll add something to the docs in a follow up.","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"37b41860b5937259de551b2449607e93cfa8b11f","unresolved":false,"context_lines":[{"line_number":5041,"context_line":"            return self.driver.set_host_enabled(enabled)"},{"line_number":5042,"context_line":"        except NotImplementedError:"},{"line_number":5043,"context_line":"            # Only the xenapi driver implements set_host_enabled but we don\u0027t"},{"line_number":5044,"context_line":"            # want NotImplementedError to get raised back to the API. We still"},{"line_number":5045,"context_line":"            # need to honor the compute RPC API contract and return \u0027enabled\u0027"},{"line_number":5046,"context_line":"            # or \u0027disabled\u0027 though."},{"line_number":5047,"context_line":"            return \u0027enabled\u0027 if enabled else \u0027disabled\u0027"},{"line_number":5048,"context_line":""},{"line_number":5049,"context_line":"    @wrap_exception()"},{"line_number":5050,"context_line":"    def get_host_uptime(self, context):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_41add699","line":5047,"range":{"start_line":5044,"start_character":70,"end_line":5047,"end_character":55},"updated":"2019-07-02 19:19:48.000000000","message":"Actually maybe it would be better to return None so that the os-hosts API would still fail for non-xenapi drivers:\n\nhttps://github.com/openstack/nova/blob/9b98bbd3121bece7ea1ee7b9dd7f910f8f418cdc/nova/api/openstack/compute/hosts.py#L173\n\nOtherwise if you were using microversion 2.1 with the libvirt driver before this change (or on an old compute) and tried that API, you\u0027d get a 501 error:\n\nhttps://github.com/openstack/nova/blob/9b98bbd3121bece7ea1ee7b9dd7f910f8f418cdc/nova/api/openstack/compute/hosts.py#L168\n\nWith this change as-is with v2.1 and the libvirt driver it would now pass which is an API behavior change. If we return None you\u0027d get a 400 back which is at least still an error.\n\nWe could do something else (hacky) and return \"not_implemented\" or something and have the API check for that and continue to return 501.","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"81ef764be7fdcfcdbcc3568f05b72c1ec6c7d27d","unresolved":false,"context_lines":[{"line_number":5041,"context_line":"            return self.driver.set_host_enabled(enabled)"},{"line_number":5042,"context_line":"        except NotImplementedError:"},{"line_number":5043,"context_line":"            # Only the xenapi driver implements set_host_enabled but we don\u0027t"},{"line_number":5044,"context_line":"            # want NotImplementedError to get raised back to the API. We still"},{"line_number":5045,"context_line":"            # need to honor the compute RPC API contract and return \u0027enabled\u0027"},{"line_number":5046,"context_line":"            # or \u0027disabled\u0027 though."},{"line_number":5047,"context_line":"            return \u0027enabled\u0027 if enabled else \u0027disabled\u0027"},{"line_number":5048,"context_line":""},{"line_number":5049,"context_line":"    @wrap_exception()"},{"line_number":5050,"context_line":"    def get_host_uptime(self, context):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_e1696a26","line":5047,"range":{"start_line":5044,"start_character":70,"end_line":5047,"end_character":55},"in_reply_to":"9fb8cfa7_41add699","updated":"2019-07-02 19:40:04.000000000","message":"I don\u0027t think that an interface that returns 501 if something isn\u0027t supported by a given driver is \"changing\" by starting to work if that implementation is upgraded to support the thing. The user here, despite being admin-y, shouldn\u0027t really know that \"the host they\u0027re operating on is libvirt, and that libvirt doesn\u0027t support this thing, and that means it should always return 501.\" This would be like saying \"well, as of 2.123, hyper-v didn\u0027t support volume attach, so we can\u0027t ever allow 2.123 to do volume-attach on hyper-v, the user has to request 2.456 to get that.\"\n\nIt sounds like the concern is that all drivers will now appear to support this operation because we\u0027re always going to return enabled or disabled and never bubble up the exception, right? I guess that just doesn\u0027t seem like a problem to me as long as the result is the same. Meaning, nobody should depend on getting a 501 for trying and failing to disable a host that can never be disabled, if we\u0027re now able to disable them all generically.\n\nSo yeah, I think what you have here is fine, IMHO.","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9b15f48eeafcbc26a1482e5e666cc46249f3245a","unresolved":false,"context_lines":[{"line_number":5041,"context_line":"            return self.driver.set_host_enabled(enabled)"},{"line_number":5042,"context_line":"        except NotImplementedError:"},{"line_number":5043,"context_line":"            # Only the xenapi driver implements set_host_enabled but we don\u0027t"},{"line_number":5044,"context_line":"            # want NotImplementedError to get raised back to the API. We still"},{"line_number":5045,"context_line":"            # need to honor the compute RPC API contract and return \u0027enabled\u0027"},{"line_number":5046,"context_line":"            # or \u0027disabled\u0027 though."},{"line_number":5047,"context_line":"            return \u0027enabled\u0027 if enabled else \u0027disabled\u0027"},{"line_number":5048,"context_line":""},{"line_number":5049,"context_line":"    @wrap_exception()"},{"line_number":5050,"context_line":"    def get_host_uptime(self, context):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_de2c2949","line":5047,"range":{"start_line":5044,"start_character":70,"end_line":5047,"end_character":55},"in_reply_to":"9fb8cfa7_e1696a26","updated":"2019-07-05 16:13:45.000000000","message":"Agreed with Dan. HTTP 5xx errors are bugs and just because we\u0027re fixing it here by accident doesn\u0027t mean it shouldn\u0027t be fixed. What we _could_ do is have a patch before this that simply caught the NotImplementedError and backported that, but given what you\u0027ve said in the commit message about the deprecation and general uselessness of the API, I don\u0027t think it\u0027s worth the effort","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9b15f48eeafcbc26a1482e5e666cc46249f3245a","unresolved":false,"context_lines":[{"line_number":9130,"context_line":"            mock.patch.object(self.compute.driver, \u0027set_host_enabled\u0027,"},{"line_number":9131,"context_line":"                              # The driver is not called in this case."},{"line_number":9132,"context_line":"                              new_callable\u003dmock.NonCallableMock),"},{"line_number":9133,"context_line":"        ) as ("},{"line_number":9134,"context_line":"            ucps, driver_set_host_enabled,"},{"line_number":9135,"context_line":"        ):"},{"line_number":9136,"context_line":"            self.assertRaises(error,"},{"line_number":9137,"context_line":"                              self.compute.set_host_enabled,"},{"line_number":9138,"context_line":"                              self.context, False)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_9e1eb10a","line":9135,"range":{"start_line":9133,"start_character":13,"end_line":9135,"end_character":10},"updated":"2019-07-05 16:13:45.000000000","message":"style nit: Could have folded these onto the same line","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9b15f48eeafcbc26a1482e5e666cc46249f3245a","unresolved":false,"context_lines":[{"line_number":9133,"context_line":"        ) as ("},{"line_number":9134,"context_line":"            ucps, driver_set_host_enabled,"},{"line_number":9135,"context_line":"        ):"},{"line_number":9136,"context_line":"            self.assertRaises(error,"},{"line_number":9137,"context_line":"                              self.compute.set_host_enabled,"},{"line_number":9138,"context_line":"                              self.context, False)"},{"line_number":9139,"context_line":"            ucps.assert_called_once_with(self.context, False)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_1ed88151","line":9136,"range":{"start_line":9136,"start_character":12,"end_line":9136,"end_character":29},"updated":"2019-07-05 16:13:45.000000000","message":"Completely unrelated, but I wished testtools provided a decorator variant of this :(","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"28cb8b48518ff2301136f53a5caa3cfa64fcf7ef","unresolved":false,"context_lines":[{"line_number":9133,"context_line":"        ) as ("},{"line_number":9134,"context_line":"            ucps, driver_set_host_enabled,"},{"line_number":9135,"context_line":"        ):"},{"line_number":9136,"context_line":"            self.assertRaises(error,"},{"line_number":9137,"context_line":"                              self.compute.set_host_enabled,"},{"line_number":9138,"context_line":"                              self.context, False)"},{"line_number":9139,"context_line":"            ucps.assert_called_once_with(self.context, False)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_5e7d99af","line":9136,"range":{"start_line":9136,"start_character":12,"end_line":9136,"end_character":29},"in_reply_to":"7faddb67_1ed88151","updated":"2019-07-05 16:46:38.000000000","message":"It does:\n\nhttps://testtools.readthedocs.io/en/latest/api.html#testtools.ExpectedException","commit_id":"099b490c2f5201732b97fbcc2560892086f32038"}]}
