)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"3d5d24f3e8cda95004023036cf525d6240d71014","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Roman Dobosz \u003croman.dobosz@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-06-29 07:55:07 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactoring of the os-services update method."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I529b585efc4d7e775e5a60f9a3e47d204aa1732d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba3cc151_12dd3f42","line":7,"updated":"2015-06-29 14:54:42.000000000","message":"And an extra \u0027.\u0027 in the title","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"a4548ca87257474b23704e932af71097a40685e2","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2015-06-29 07:55:07 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactoring of the os-services update method."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I529b585efc4d7e775e5a60f9a3e47d204aa1732d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba3cc151_2f3a6037","line":8,"updated":"2015-06-29 14:40:01.000000000","message":"At least say something?","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4e07b1a154d8f7058460117b193ef9181b7195f5","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2015-06-29 07:55:07 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactoring of the os-services update method."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I529b585efc4d7e775e5a60f9a3e47d204aa1732d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba3cc151_c652afdd","line":8,"in_reply_to":"ba3cc151_2f3a6037","updated":"2015-06-29 20:15:00.000000000","message":"Could you please explain why this change is needed ?","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"6f7d2c2f3f1db2ae97272ea1371bad505da1b40c","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2015-06-29 07:55:07 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactoring of the os-services update method."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I529b585efc4d7e775e5a60f9a3e47d204aa1732d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba3cc151_6944c473","line":8,"in_reply_to":"ba3cc151_c652afdd","updated":"2015-06-30 07:24:40.000000000","message":"Yes, sorry. Will do in next patch set.","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4e07b1a154d8f7058460117b193ef9181b7195f5","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactoring of the os-services update method."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I529b585efc4d7e775e5a60f9a3e47d204aa1732d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba3cc151_86d58744","line":9,"updated":"2015-06-29 20:15:00.000000000","message":"Since the patch is needed for the blueprint, could you please add a :\n\nPartially-Implements: blueprint mark-host-down \n\n?","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"6f7d2c2f3f1db2ae97272ea1371bad505da1b40c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactoring of the os-services update method."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I529b585efc4d7e775e5a60f9a3e47d204aa1732d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba3cc151_e958d458","line":9,"in_reply_to":"ba3cc151_86d58744","updated":"2015-06-30 07:24:40.000000000","message":"Definitely.","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"}],"nova/api/openstack/compute/plugins/v3/services.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4e07b1a154d8f7058460117b193ef9181b7195f5","unresolved":false,"context_lines":[{"line_number":64,"context_line":"                          \u0027zone\u0027: svc[\u0027availability_zone\u0027],"},{"line_number":65,"context_line":"                          \u0027status\u0027: active, \u0027state\u0027: state,"},{"line_number":66,"context_line":"                          \u0027updated_at\u0027: svc[\u0027updated_at\u0027],"},{"line_number":67,"context_line":"                          \u0027disabled_reason\u0027: svc[\u0027disabled_reason\u0027]}"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        return service_detail"},{"line_number":70,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_6605e3c0","line":67,"updated":"2015-06-29 20:15:00.000000000","message":"all of these whitespace changes are unnecessary","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"e4d9ce52d4e2d9abf08557723aa82976ff897224","unresolved":false,"context_lines":[{"line_number":64,"context_line":"                          \u0027zone\u0027: svc[\u0027availability_zone\u0027],"},{"line_number":65,"context_line":"                          \u0027status\u0027: active, \u0027state\u0027: state,"},{"line_number":66,"context_line":"                          \u0027updated_at\u0027: svc[\u0027updated_at\u0027],"},{"line_number":67,"context_line":"                          \u0027disabled_reason\u0027: svc[\u0027disabled_reason\u0027]}"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        return service_detail"},{"line_number":70,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_f8e7403e","line":67,"in_reply_to":"ba3cc151_6605e3c0","updated":"2015-06-30 07:12:38.000000000","message":"This patch is about refactoring, right? Why not change this as well?","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"3bb98f15439ca5193803701291b6ec4dd6fe7798","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        return self._disable(body, context, reason)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def _enable_disable(self, body, context, status, payload):"},{"line_number":102,"context_line":"        \"\"\"Enable/Disable scheduling for a service.\"\"\""},{"line_number":103,"context_line":"        reason \u003d payload.get(\u0027disabled_reason\u0027)"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_dc8930cc","line":101,"updated":"2015-06-29 14:26:03.000000000","message":"nit: s/enable_disable/_toggle_service","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a79d9ee8f344be55e691f963e928bbba7b200665","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        return self._disable(body, context, reason)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def _enable_disable(self, body, context, status, payload):"},{"line_number":102,"context_line":"        \"\"\"Enable/Disable scheduling for a service.\"\"\""},{"line_number":103,"context_line":"        reason \u003d payload.get(\u0027disabled_reason\u0027)"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_39ecdcc6","line":101,"in_reply_to":"ba3cc151_26b03bdd","updated":"2015-06-29 20:31:27.000000000","message":"toggle_service is not right either. enable_disable makes plenty of sense to me in the current usage, FWIW.","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"fcac7953caeeebc23fa8ccf9453a26950721d16a","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        return self._disable(body, context, reason)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def _enable_disable(self, body, context, status, payload):"},{"line_number":102,"context_line":"        \"\"\"Enable/Disable scheduling for a service.\"\"\""},{"line_number":103,"context_line":"        reason \u003d payload.get(\u0027disabled_reason\u0027)"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_247d811c","line":101,"in_reply_to":"ba3cc151_39ecdcc6","updated":"2015-06-30 07:45:00.000000000","message":"Done","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4e07b1a154d8f7058460117b193ef9181b7195f5","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        return self._disable(body, context, reason)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def _enable_disable(self, body, context, status, payload):"},{"line_number":102,"context_line":"        \"\"\"Enable/Disable scheduling for a service.\"\"\""},{"line_number":103,"context_line":"        reason \u003d payload.get(\u0027disabled_reason\u0027)"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_26b03bdd","line":101,"in_reply_to":"ba3cc151_dc8930cc","updated":"2015-06-29 20:15:00.000000000","message":"agreed, that\u0027s a very bad proxy name","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4e07b1a154d8f7058460117b193ef9181b7195f5","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        self._update(context, body[\u0027host\u0027], body[\u0027binary\u0027], payload)"},{"line_number":117,"context_line":"        return ret_value"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def _update(self, context, host, binary, payload):"},{"line_number":120,"context_line":"        \"\"\"Do the actual PUT/update\"\"\""},{"line_number":121,"context_line":"        try:"},{"line_number":122,"context_line":"            self.host_api.service_update(context, host, binary, payload)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_46ed3f8a","line":119,"updated":"2015-06-29 20:15:00.000000000","message":"I don\u0027t really see why you need so many cascaded calls...","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"e4d9ce52d4e2d9abf08557723aa82976ff897224","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        self._update(context, body[\u0027host\u0027], body[\u0027binary\u0027], payload)"},{"line_number":117,"context_line":"        return ret_value"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def _update(self, context, host, binary, payload):"},{"line_number":120,"context_line":"        \"\"\"Do the actual PUT/update\"\"\""},{"line_number":121,"context_line":"        try:"},{"line_number":122,"context_line":"            self.host_api.service_update(context, host, binary, payload)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_d8bfdcce","line":119,"in_reply_to":"ba3cc151_46ed3f8a","updated":"2015-06-30 07:12:38.000000000","message":"If someone is going to add another action for the update/PUT (and it\u0027s what I\u0027m going to do in the last patchset), there would be a copy-paste of lines 121-124. OTOH having everything in one big function (update in this case) might be problematic in case of providing another action or actions.","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e54beca27c1ef526993cd4a86afda61c95b75513","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        self._update(context, body[\u0027host\u0027], body[\u0027binary\u0027], payload)"},{"line_number":117,"context_line":"        return ret_value"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def _update(self, context, host, binary, payload):"},{"line_number":120,"context_line":"        \"\"\"Do the actual PUT/update\"\"\""},{"line_number":121,"context_line":"        try:"},{"line_number":122,"context_line":"            self.host_api.service_update(context, host, binary, payload)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_2986aa13","line":119,"in_reply_to":"ba3cc151_d8bfdcce","updated":"2015-06-30 14:25:30.000000000","message":"Oh right, you\u0027re factoring _update(), missed it","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4e07b1a154d8f7058460117b193ef9181b7195f5","unresolved":false,"context_lines":[{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        try:"},{"line_number":160,"context_line":"            action \u003d actions[id]"},{"line_number":161,"context_line":"        except KeyError:"},{"line_number":162,"context_line":"            msg \u003d _(\"Unknown action\")"},{"line_number":163,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_a6f1abcf","line":161,"updated":"2015-06-29 20:15:00.000000000","message":"Well, I don\u0027t want to nitpick but you can usually do that this way :\n\n  action \u003d actions.get(id)\n  if not action:\n    raise sth()","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a79d9ee8f344be55e691f963e928bbba7b200665","unresolved":false,"context_lines":[{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        try:"},{"line_number":160,"context_line":"            action \u003d actions[id]"},{"line_number":161,"context_line":"        except KeyError:"},{"line_number":162,"context_line":"            msg \u003d _(\"Unknown action\")"},{"line_number":163,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_994968d3","line":161,"in_reply_to":"ba3cc151_a6f1abcf","updated":"2015-06-29 20:31:27.000000000","message":"FWIW, I prefer what he has here. Either works of course.","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"e4d9ce52d4e2d9abf08557723aa82976ff897224","unresolved":false,"context_lines":[{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        try:"},{"line_number":160,"context_line":"            action \u003d actions[id]"},{"line_number":161,"context_line":"        except KeyError:"},{"line_number":162,"context_line":"            msg \u003d _(\"Unknown action\")"},{"line_number":163,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_4b38d8d3","line":161,"in_reply_to":"ba3cc151_a6f1abcf","updated":"2015-06-30 07:12:38.000000000","message":"Sylvain, I\u0027m sorry, but I have to disagree. In this case the EAFP technique applies[1].\n\n[1] https://docs.python.org/2/glossary.html#term-eafp","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"3bb98f15439ca5193803701291b6ec4dd6fe7798","unresolved":false,"context_lines":[{"line_number":162,"context_line":"            msg \u003d _(\"Unknown action\")"},{"line_number":163,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        return action(body, context)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"class Services(extensions.V3APIExtensionBase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_ef10f850","line":165,"updated":"2015-06-29 14:26:03.000000000","message":"You will never pass the disable reason here","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a79d9ee8f344be55e691f963e928bbba7b200665","unresolved":false,"context_lines":[{"line_number":162,"context_line":"            msg \u003d _(\"Unknown action\")"},{"line_number":163,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        return action(body, context)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"class Services(extensions.V3APIExtensionBase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_992088f9","line":165,"in_reply_to":"ba3cc151_06a17794","updated":"2015-06-29 20:31:27.000000000","message":"Right, I think this is fine as it is, unless I\u0027m missing something.","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e54beca27c1ef526993cd4a86afda61c95b75513","unresolved":false,"context_lines":[{"line_number":162,"context_line":"            msg \u003d _(\"Unknown action\")"},{"line_number":163,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        return action(body, context)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"class Services(extensions.V3APIExtensionBase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_e9ca026e","line":165,"in_reply_to":"ba3cc151_126d3f04","updated":"2015-06-30 14:25:30.000000000","message":"Okay, I see your pattern. IMHO, it\u0027s a bit bizarre to me but okay... If I dislike it, I can just provide another patch for modifying it.","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"fcac7953caeeebc23fa8ccf9453a26950721d16a","unresolved":false,"context_lines":[{"line_number":162,"context_line":"            msg \u003d _(\"Unknown action\")"},{"line_number":163,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        return action(body, context)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"class Services(extensions.V3APIExtensionBase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_e49f995d","line":165,"in_reply_to":"ba3cc151_126d3f04","updated":"2015-06-30 07:45:00.000000000","message":"Yes my mistake.","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"e4d9ce52d4e2d9abf08557723aa82976ff897224","unresolved":false,"context_lines":[{"line_number":162,"context_line":"            msg \u003d _(\"Unknown action\")"},{"line_number":163,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        return action(body, context)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"class Services(extensions.V3APIExtensionBase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_126d3f04","line":165,"in_reply_to":"ba3cc151_ef10f850","updated":"2015-06-30 07:12:38.000000000","message":"Sahid, Disabled reason is not passed here directly, It\u0027s extracted from body in _disable_log_reason method.\n\nSylvain, the concept here is, that all methods for enable/disable have the same signature except disable method, which have one optional argument - reason. The method disable_log_reason differ from disable method only by the fact, that it requires reason to be provided.","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4e07b1a154d8f7058460117b193ef9181b7195f5","unresolved":false,"context_lines":[{"line_number":162,"context_line":"            msg \u003d _(\"Unknown action\")"},{"line_number":163,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        return action(body, context)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"class Services(extensions.V3APIExtensionBase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_06a17794","line":165,"in_reply_to":"ba3cc151_ef10f850","updated":"2015-06-29 20:15:00.000000000","message":"well, the problem here is that you don\u0027t have the same signature for all your new methods. I\u0027d rather have the same Facade for both disable and disable-log-reason which would make the code quite identical as the original one.","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9a77999d3c3be6560b12c3336574625d10869837","unresolved":false,"context_lines":[{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    def get_resources(self):"},{"line_number":176,"context_line":"        resources \u003d [extensions.ResourceExtension(ALIAS,"},{"line_number":177,"context_line":"                                                  ServiceController())]"},{"line_number":178,"context_line":"        return resources"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    def get_controller_extensions(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_3c037c75","line":177,"updated":"2015-06-29 14:20:38.000000000","message":"This is an unrelated change, please don\u0027t make random whitespace changes.","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"e4d9ce52d4e2d9abf08557723aa82976ff897224","unresolved":false,"context_lines":[{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    def get_resources(self):"},{"line_number":176,"context_line":"        resources \u003d [extensions.ResourceExtension(ALIAS,"},{"line_number":177,"context_line":"                                                  ServiceController())]"},{"line_number":178,"context_line":"        return resources"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    def get_controller_extensions(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_bc7eacac","line":177,"in_reply_to":"ba3cc151_3c037c75","updated":"2015-06-30 07:12:38.000000000","message":"This line is misaligned, and as I\u0027m doing refactoring, why to skip fix for that line?","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4e07b1a154d8f7058460117b193ef9181b7195f5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ba3cc151_e6db33ec","line":182,"updated":"2015-06-29 20:15:00.000000000","message":"You don\u0027t really need to modify the tests since it\u0027s just pure internal refactoring, but since the current TestCase is not really mocking the right thing (but instead just mocks the DB), I wonder if it would be a good opportunity for just adding a few tests using mock.assert_called_once_with() where the mocked method could be self.host_api.service_update","commit_id":"7abbc5bc507f42c99e44be713efb3b0325b64362"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2c6bbe5b93d07555414befeb3084423a3ae88967","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        except exception.HostBinaryNotFound as exc:"},{"line_number":120,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dexc.format_message())"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    def _enable(self, body, context):"},{"line_number":123,"context_line":"        \"\"\"Enable scheduling for a service.\"\"\""},{"line_number":124,"context_line":"        return self._toggle_service(body, context, \"enabled\","},{"line_number":125,"context_line":"                                    {\u0027disabled\u0027: False,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba3cc151_09cba6a6","line":122,"updated":"2015-06-30 14:25:15.000000000","message":"is it me or you messed up something when rebasing ? what implementation is correct ? This one or the above one ?","commit_id":"517f46b1cd782251c7c42fd736144c148181e5ac"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"e6c72c5877eea26651be2240173bd03ff26bc6df","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        except exception.HostBinaryNotFound as exc:"},{"line_number":120,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dexc.format_message())"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    def _enable(self, body, context):"},{"line_number":123,"context_line":"        \"\"\"Enable scheduling for a service.\"\"\""},{"line_number":124,"context_line":"        return self._toggle_service(body, context, \"enabled\","},{"line_number":125,"context_line":"                                    {\u0027disabled\u0027: False,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba3cc151_5347144b","line":122,"in_reply_to":"ba3cc151_09cba6a6","updated":"2015-06-30 15:34:44.000000000","message":"Oh man... The above one should be used.","commit_id":"517f46b1cd782251c7c42fd736144c148181e5ac"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"ada3a29413a566cf387d837e53020a7d594ac837","unresolved":false,"context_lines":[{"line_number":84,"context_line":"                                    {\u0027disabled\u0027: True,"},{"line_number":85,"context_line":"                                     \u0027disabled_reason\u0027: reason})"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def _disable_log_reason(self, body, context):"},{"line_number":88,"context_line":"        \"\"\"Disable scheduling for a service with a log.\"\"\""},{"line_number":89,"context_line":"        try:"},{"line_number":90,"context_line":"            reason \u003d body[\u0027disabled_reason\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_cf303b04","line":87,"updated":"2015-07-01 05:59:43.000000000","message":"disabled_reqson is not required, that is optional.\nthis validation is wrong.\n\nIf you want to do that on a new microversion, we need to define it as required on json-schema of the microversion.\n\nhttps://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/v3/services.py#L28","commit_id":"7cef4e95beaac0dca4ae60b04aac6b767c183552"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"16cd86d978621c14b2d9c8716023986b33836348","unresolved":false,"context_lines":[{"line_number":84,"context_line":"                                    {\u0027disabled\u0027: True,"},{"line_number":85,"context_line":"                                     \u0027disabled_reason\u0027: reason})"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def _disable_log_reason(self, body, context):"},{"line_number":88,"context_line":"        \"\"\"Disable scheduling for a service with a log.\"\"\""},{"line_number":89,"context_line":"        try:"},{"line_number":90,"context_line":"            reason \u003d body[\u0027disabled_reason\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_d35f4fe3","line":87,"in_reply_to":"ba3cc151_58ca884f","updated":"2015-07-02 00:45:38.000000000","message":"@Roman\n\nI\u0027m sorry, I completely did misread the code.\nI understand this part is completely right.","commit_id":"7cef4e95beaac0dca4ae60b04aac6b767c183552"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"57e83613c38f2ac2971eda8522ad3d1261b82fea","unresolved":false,"context_lines":[{"line_number":84,"context_line":"                                    {\u0027disabled\u0027: True,"},{"line_number":85,"context_line":"                                     \u0027disabled_reason\u0027: reason})"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def _disable_log_reason(self, body, context):"},{"line_number":88,"context_line":"        \"\"\"Disable scheduling for a service with a log.\"\"\""},{"line_number":89,"context_line":"        try:"},{"line_number":90,"context_line":"            reason \u003d body[\u0027disabled_reason\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_58ca884f","line":87,"in_reply_to":"ba3cc151_cf303b04","updated":"2015-07-01 19:36:06.000000000","message":"Kenichi, than, it\u0027s looks like the original implementation was wrong[1], since it raises HTTPBadRequest while calling \u0027disable-log-reason\u0027 action without the reason in payload.\n\nEither way is okay with me: introducing the microversion or changing original behavior to align with the schema. Question is should it be a separate bug report or as a new patch to mark-host-down patchset.\n\n[1] https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/plugins/v3/services.py#L135","commit_id":"7cef4e95beaac0dca4ae60b04aac6b767c183552"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"16cd86d978621c14b2d9c8716023986b33836348","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        return self._disable(body, context, reason)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _enable_disable(self, body, context, status, payload):"},{"line_number":98,"context_line":"        \"\"\"Enable/Disable scheduling for a service.\"\"\""},{"line_number":99,"context_line":"        reason \u003d payload.get(\u0027disabled_reason\u0027)"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_73d0c3f6","line":97,"updated":"2015-07-02 00:45:38.000000000","message":"The arguments \"body\" and \"payload\" are a little difficult to read, because the meanings are similar.\n\nHow about passing host and binary instead of body ?\n\n e.g:\n\n def _enable_disable(self, context, host, binary, status, payload):\n     \"\"\"Enable/Disable scheduling for a service.\"\"\"\n\nIf doing that, we _enable_disable() doesn\u0027t need to contain \"body[..]\" multiple times.","commit_id":"7cef4e95beaac0dca4ae60b04aac6b767c183552"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"37fe37ae04bf3ffdccfe40c2fd8aed0b3f53518d","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        return self._disable(body, context, reason)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _enable_disable(self, body, context, status, payload):"},{"line_number":98,"context_line":"        \"\"\"Enable/Disable scheduling for a service.\"\"\""},{"line_number":99,"context_line":"        reason \u003d payload.get(\u0027disabled_reason\u0027)"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_a7ed9eb8","line":97,"in_reply_to":"ba3cc151_04f1e800","updated":"2015-07-02 07:48:08.000000000","message":"ah, params_to_update seems nice for me.\nThanks for the suggestion.","commit_id":"7cef4e95beaac0dca4ae60b04aac6b767c183552"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"4ca49185d2463c209d849a472295e276a4326881","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        return self._disable(body, context, reason)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _enable_disable(self, body, context, status, payload):"},{"line_number":98,"context_line":"        \"\"\"Enable/Disable scheduling for a service.\"\"\""},{"line_number":99,"context_line":"        reason \u003d payload.get(\u0027disabled_reason\u0027)"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_04f1e800","line":97,"in_reply_to":"ba3cc151_73d0c3f6","updated":"2015-07-02 07:08:37.000000000","message":"The difference between body and the payload here is body origins from request and the payload will be the params for service_update on HostAPI object. Maybe it\u0027s worth to change \u0027payload\u0027 with \u0027params_to_update\u0027 to avoid confusion?","commit_id":"7cef4e95beaac0dca4ae60b04aac6b767c183552"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"d694f5b4ad13a8bd660c0f6f2c0f718561074547","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        return self._disable(body, context, reason)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _enable_disable(self, body, context, status, payload):"},{"line_number":98,"context_line":"        \"\"\"Enable/Disable scheduling for a service.\"\"\""},{"line_number":99,"context_line":"        reason \u003d payload.get(\u0027disabled_reason\u0027)"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_2db88fc7","line":97,"in_reply_to":"ba3cc151_a7ed9eb8","updated":"2015-07-02 08:43:12.000000000","message":"Ok. I\u0027ll change it than.","commit_id":"7cef4e95beaac0dca4ae60b04aac6b767c183552"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"16cd86d978621c14b2d9c8716023986b33836348","unresolved":false,"context_lines":[{"line_number":156,"context_line":"            msg \u003d _(\"Unknown action\")"},{"line_number":157,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        return action(body, context)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"class Services(extensions.V3APIExtensionBase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_f6884173","line":159,"updated":"2015-07-02 00:45:38.000000000","message":"If line97 suggestion is acceptable, here can be\n\n return action(context, body[\u0027host\u0027], body[\u0027binary\u0027], body)","commit_id":"7cef4e95beaac0dca4ae60b04aac6b767c183552"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"d694f5b4ad13a8bd660c0f6f2c0f718561074547","unresolved":false,"context_lines":[{"line_number":156,"context_line":"            msg \u003d _(\"Unknown action\")"},{"line_number":157,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        return action(body, context)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"class Services(extensions.V3APIExtensionBase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_0d710ba8","line":159,"in_reply_to":"ba3cc151_4762923f","updated":"2015-07-02 08:43:12.000000000","message":"Ok :)","commit_id":"7cef4e95beaac0dca4ae60b04aac6b767c183552"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"37fe37ae04bf3ffdccfe40c2fd8aed0b3f53518d","unresolved":false,"context_lines":[{"line_number":156,"context_line":"            msg \u003d _(\"Unknown action\")"},{"line_number":157,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        return action(body, context)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"class Services(extensions.V3APIExtensionBase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_4762923f","line":159,"in_reply_to":"ba3cc151_de12ef56","updated":"2015-07-02 07:48:08.000000000","message":"That was just readability but I see your point. that seems redundant.\n\nThe above suggestion is nice for me, so I drop the above my comment.","commit_id":"7cef4e95beaac0dca4ae60b04aac6b767c183552"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"4ca49185d2463c209d849a472295e276a4326881","unresolved":false,"context_lines":[{"line_number":156,"context_line":"            msg \u003d _(\"Unknown action\")"},{"line_number":157,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        return action(body, context)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"class Services(extensions.V3APIExtensionBase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_de12ef56","line":159,"in_reply_to":"ba3cc151_f6884173","updated":"2015-07-02 07:08:37.000000000","message":"Kenichi, this is exactly what I\u0027d like to avoid - if there is host and binary in the body, why not simply pass just the body?","commit_id":"7cef4e95beaac0dca4ae60b04aac6b767c183552"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5297a1aba637d27c6fe8208f33034ee6c0ec7606","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    def _get_services(self, req):"},{"line_number":37,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"},{"line_number":38,"context_line":"        authorize(context)"},{"line_number":39,"context_line":"        _services \u003d self.host_api.service_get_all(context, set_zones\u003dTrue)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"        host \u003d \u0027\u0027"},{"line_number":42,"context_line":"        if \u0027host\u0027 in req.GET:"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_77bd39d6","line":39,"updated":"2015-07-07 22:32:46.000000000","message":"is there a reason for making this vaiable \"private\" by pre-pending with an underscore?","commit_id":"e03404cdefcb043ac545af31574202eb2ee6b544"}]}