)]}'
{"nova/virt/ironic/client_wrapper.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3abab5419c6e0546da513941b7c7ac0fa0600d73","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        :param args: Client method arguments."},{"line_number":116,"context_line":"        :param kwargs: Client method keyword arguments."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        Optional arguments:"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        :param retriable: Boolean value. Whether the request should be"},{"line_number":121,"context_line":"                          retried or not. Defaults to True."}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_7d7f4be0","line":118,"updated":"2015-09-23 08:23:51.000000000","message":"nit: the subheading is not required, just continue with a new :param:, otherwise, I suppose, it won\u0027t get rendered properly.","commit_id":"2a37b33654eb9de2cfd4849bdcf13b4e0318e44b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d640d788108fad211460bb74802481b0ef5f90bd","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        :param args: Client method arguments."},{"line_number":116,"context_line":"        :param kwargs: Client method keyword arguments."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        Optional arguments:"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        :param retriable: Boolean value. Whether the request should be"},{"line_number":121,"context_line":"                          retried or not. Defaults to True."}],"source_content_type":"text/x-python","patch_set":1,"id":"9a8ffd7b_238787d8","line":118,"in_reply_to":"ba15a1d1_7d7f4be0","updated":"2015-11-26 09:40:33.000000000","message":"Done","commit_id":"2a37b33654eb9de2cfd4849bdcf13b4e0318e44b"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3abab5419c6e0546da513941b7c7ac0fa0600d73","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        Optional arguments:"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        :param retriable: Boolean value. Whether the request should be"},{"line_number":121,"context_line":"                          retried or not. Defaults to True."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"        :raises: NovaException if all retries failed."},{"line_number":124,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_dd6e972e","line":121,"updated":"2015-09-23 08:23:51.000000000","message":"this should note that it won\u0027t use a cached client","commit_id":"2a37b33654eb9de2cfd4849bdcf13b4e0318e44b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d640d788108fad211460bb74802481b0ef5f90bd","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        Optional arguments:"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        :param retriable: Boolean value. Whether the request should be"},{"line_number":121,"context_line":"                          retried or not. Defaults to True."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"        :raises: NovaException if all retries failed."},{"line_number":124,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a8ffd7b_8378b3f3","line":121,"in_reply_to":"ba15a1d1_dd6e972e","updated":"2015-11-26 09:40:33.000000000","message":"Done","commit_id":"2a37b33654eb9de2cfd4849bdcf13b4e0318e44b"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3abab5419c6e0546da513941b7c7ac0fa0600d73","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        retriable \u003d kwargs.pop(\u0027retriable\u0027, True)"},{"line_number":131,"context_line":"        max_retries \u003d CONF.ironic.api_max_retries if retriable else 1"},{"line_number":132,"context_line":"        retry_interval \u003d CONF.ironic.api_retry_interval if retriable else 0"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        # num_attempts should be the times of retry + 1"},{"line_number":135,"context_line":"        # eg. retry\u003d\u003d0 just means  run once and no retry"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_3d0eb38c","line":132,"updated":"2015-09-23 08:23:51.000000000","message":"this change is not needed: retries in this function are for connection errors, which we still have to retry actually","commit_id":"2a37b33654eb9de2cfd4849bdcf13b4e0318e44b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d640d788108fad211460bb74802481b0ef5f90bd","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        retriable \u003d kwargs.pop(\u0027retriable\u0027, True)"},{"line_number":131,"context_line":"        max_retries \u003d CONF.ironic.api_max_retries if retriable else 1"},{"line_number":132,"context_line":"        retry_interval \u003d CONF.ironic.api_retry_interval if retriable else 0"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        # num_attempts should be the times of retry + 1"},{"line_number":135,"context_line":"        # eg. retry\u003d\u003d0 just means  run once and no retry"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a8ffd7b_e3509f60","line":132,"in_reply_to":"ba15a1d1_3d0eb38c","updated":"2015-11-26 09:40:33.000000000","message":"Done","commit_id":"2a37b33654eb9de2cfd4849bdcf13b4e0318e44b"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b484d1dcece31ad2170d51dfa4e6faaa1c322c2c","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        \"\"\"Tell the wrapper to invalidate the cached ironic-client.\"\"\""},{"line_number":57,"context_line":"        self._cached_client \u003d None"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def _get_client(self, retriable\u003dTrue):"},{"line_number":60,"context_line":"        max_retries \u003d CONF.ironic.api_max_retries if retriable else 1"},{"line_number":61,"context_line":"        retry_interval \u003d CONF.ironic.api_retry_interval if retriable else 0"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a8ffd7b_509f2a5f","line":59,"range":{"start_line":59,"start_character":26,"end_line":59,"end_character":35},"updated":"2015-12-01 13:28:36.000000000","message":"So I guess the only complain I have after the last explanation (and thanks for that btw) is that we should name this better. retry_on_conflict paints a much clearer picture, even though the naming in the ironic client also sux.","commit_id":"8f47ed9e19ac96b2074c5b058de0bcd7557be5d5"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"83d98fa249cae9221940693e506fc99f09976702","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        \"\"\"Tell the wrapper to invalidate the cached ironic-client.\"\"\""},{"line_number":57,"context_line":"        self._cached_client \u003d None"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def _get_client(self, retriable\u003dTrue):"},{"line_number":60,"context_line":"        max_retries \u003d CONF.ironic.api_max_retries if retriable else 1"},{"line_number":61,"context_line":"        retry_interval \u003d CONF.ironic.api_retry_interval if retriable else 0"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a8ffd7b_30caa61b","line":59,"in_reply_to":"9a8ffd7b_509f2a5f","updated":"2015-12-01 13:31:32.000000000","message":"fair enough, yeah the whole retry on *all* conflict sucks.\n\nThe API in Ironic should be smarter and actually tells what is retryable and what is not by either using a different http return code or headers such as Retry-After. Unfortunately none of those are done today (we have a bug to track it)","commit_id":"8f47ed9e19ac96b2074c5b058de0bcd7557be5d5"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b484d1dcece31ad2170d51dfa4e6faaa1c322c2c","unresolved":false,"context_lines":[{"line_number":108,"context_line":"            obj \u003d getattr(obj, attribute)"},{"line_number":109,"context_line":"        return obj"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def call(self, method, *args, **kwargs):"},{"line_number":112,"context_line":"        \"\"\"Call an Ironic client method and retry on errors."},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        :param method: Name of the client method to call as a string."}],"source_content_type":"text/x-python","patch_set":2,"id":"9a8ffd7b_70bfceaa","line":111,"range":{"start_line":111,"start_character":14,"end_line":111,"end_character":42},"updated":"2015-12-01 13:28:36.000000000","message":"how about making it an explicit keyword argument?","commit_id":"8f47ed9e19ac96b2074c5b058de0bcd7557be5d5"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"b8516fd265c100687420039a74e39b0b8f94ced3","unresolved":false,"context_lines":[{"line_number":108,"context_line":"            obj \u003d getattr(obj, attribute)"},{"line_number":109,"context_line":"        return obj"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def call(self, method, *args, **kwargs):"},{"line_number":112,"context_line":"        \"\"\"Call an Ironic client method and retry on errors."},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        :param method: Name of the client method to call as a string."}],"source_content_type":"text/x-python","patch_set":2,"id":"9a8ffd7b_36029e0a","line":111,"in_reply_to":"9a8ffd7b_504e4ac2","updated":"2015-12-01 13:53:40.000000000","message":"Actually not done. By making it an explicit keyword argument* it will affect every single invocation of the call() method, if we look at the driver.py you will see we do use a lot of positional arguments to invoke call().\n\nIf possible and you agree with, I would like to keep this patch minimal\n\n * def call(self, method, retry_on_conflict\u003dTrue, *args, **kwargs):","commit_id":"8f47ed9e19ac96b2074c5b058de0bcd7557be5d5"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"83d98fa249cae9221940693e506fc99f09976702","unresolved":false,"context_lines":[{"line_number":108,"context_line":"            obj \u003d getattr(obj, attribute)"},{"line_number":109,"context_line":"        return obj"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def call(self, method, *args, **kwargs):"},{"line_number":112,"context_line":"        \"\"\"Call an Ironic client method and retry on errors."},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        :param method: Name of the client method to call as a string."}],"source_content_type":"text/x-python","patch_set":2,"id":"9a8ffd7b_504e4ac2","line":111,"in_reply_to":"9a8ffd7b_70bfceaa","updated":"2015-12-01 13:31:32.000000000","message":"Done","commit_id":"8f47ed9e19ac96b2074c5b058de0bcd7557be5d5"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b484d1dcece31ad2170d51dfa4e6faaa1c322c2c","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        :param method: Name of the client method to call as a string."},{"line_number":115,"context_line":"        :param args: Client method arguments."},{"line_number":116,"context_line":"        :param kwargs: Client method keyword arguments."},{"line_number":117,"context_line":"        :param retriable: Boolean value. Whether the request should be"},{"line_number":118,"context_line":"                          retried or not. If retriable is False the"},{"line_number":119,"context_line":"                          cached instance of the client won\u0027t be used."},{"line_number":120,"context_line":"                          Defaults to True."},{"line_number":121,"context_line":"        :raises: NovaException if all retries failed."},{"line_number":122,"context_line":"        \"\"\""},{"line_number":123,"context_line":"        # TODO(dtantsur): drop these once ironicclient 0.8.0 is out and used in"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a8ffd7b_50adea86","line":120,"range":{"start_line":117,"start_character":8,"end_line":120,"end_character":43},"updated":"2015-12-01 13:28:36.000000000","message":"Same comment as above","commit_id":"8f47ed9e19ac96b2074c5b058de0bcd7557be5d5"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"83d98fa249cae9221940693e506fc99f09976702","unresolved":false,"context_lines":[{"line_number":117,"context_line":"        :param retriable: Boolean value. Whether the request should be"},{"line_number":118,"context_line":"                          retried or not. If retriable is False the"},{"line_number":119,"context_line":"                          cached instance of the client won\u0027t be used."},{"line_number":120,"context_line":"                          Defaults to True."},{"line_number":121,"context_line":"        :raises: NovaException if all retries failed."},{"line_number":122,"context_line":"        \"\"\""},{"line_number":123,"context_line":"        # TODO(dtantsur): drop these once ironicclient 0.8.0 is out and used in"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a8ffd7b_b04f76bd","line":120,"in_reply_to":"9a8ffd7b_50adea86","updated":"2015-12-01 13:31:32.000000000","message":"Done","commit_id":"8f47ed9e19ac96b2074c5b058de0bcd7557be5d5"}],"nova/virt/ironic/driver.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"a342c1586b5a9205562530de3df50a1452c98a8e","unresolved":false,"context_lines":[{"line_number":387,"context_line":"            # pick by the scheduler is already associated with another instance"},{"line_number":388,"context_line":"            # due bug #1341420."},{"line_number":389,"context_line":"            self.ironicclient.call(\u0027node.update\u0027, node.uuid, patch,"},{"line_number":390,"context_line":"                                   retriable\u003dFalse)"},{"line_number":391,"context_line":"        except ironic.exc.BadRequest:"},{"line_number":392,"context_line":"            msg \u003d (_(\"Failed to add deploy parameters on node %(node)s \""},{"line_number":393,"context_line":"                     \"when provisioning the instance %(instance)s\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a8ffd7b_1fec468e","line":390,"range":{"start_line":390,"start_character":35,"end_line":390,"end_character":50},"updated":"2015-11-25 14:10:05.000000000","message":"So this obviously helps Ironic when there is heavy (any?) load on it, due to massive raciness of the nova scheduler.\n\nIdeally - we would have a specific return code from Ironic that would prompt us to not retry, and still do retry in case of other common failures (I have no idea if ironic API returns such an error but if it does I\u0027d be -1 on this patch).\n\nOverall this is obviously a hack - but it might be a palatable hack (due to it\u0027s minimal scope mostly) until we fix the Nova scheduler.\n\nSo I guess the question is - can we decide what to do based on the retrun status from Ironic? If yes - than lets\u0027 do that even if it requries a bit of client refactoring.","commit_id":"2a37b33654eb9de2cfd4849bdcf13b4e0318e44b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d640d788108fad211460bb74802481b0ef5f90bd","unresolved":false,"context_lines":[{"line_number":387,"context_line":"            # pick by the scheduler is already associated with another instance"},{"line_number":388,"context_line":"            # due bug #1341420."},{"line_number":389,"context_line":"            self.ironicclient.call(\u0027node.update\u0027, node.uuid, patch,"},{"line_number":390,"context_line":"                                   retriable\u003dFalse)"},{"line_number":391,"context_line":"        except ironic.exc.BadRequest:"},{"line_number":392,"context_line":"            msg \u003d (_(\"Failed to add deploy parameters on node %(node)s \""},{"line_number":393,"context_line":"                     \"when provisioning the instance %(instance)s\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a8ffd7b_437acbf2","line":390,"in_reply_to":"9a8ffd7b_1fec468e","updated":"2015-11-26 09:40:33.000000000","message":"Yes, under the hood the python-ironicclient library will retry only for HTTP 409 (Conflict) [0] [1].\n\nThis is historical and kinda sucks because clients using the Ironic API have a hard time figuring out what conflicts they can/should retry and what not. We current have a bug open [2] about it, ideally the Ironic API itself should indicate what is retry-able and what is not (Via a Retry-After header or using a different HTTP return code)\n\n[0] https://github.com/openstack/python-ironicclient/blob/1073a5469bf0407fd46ce39a9e384bfc0aa1ae1a/ironicclient/shell.py#L233-L237\n\n[1] https://github.com/openstack/nova/blob/master/nova/virt/ironic/client_wrapper.py#L78-L80\n\n[2] https://bugs.launchpad.net/ironic/+bug/1461140","commit_id":"2a37b33654eb9de2cfd4849bdcf13b4e0318e44b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"2fd097b2d848bc9c2172c5f2b09c3ff85048e5bc","unresolved":false,"context_lines":[{"line_number":397,"context_line":"            # node picked by the scheduler is already associated with another"},{"line_number":398,"context_line":"            # instance due bug #1341420."},{"line_number":399,"context_line":"            self.ironicclient.call(\u0027node.update\u0027, node.uuid, patch,"},{"line_number":400,"context_line":"                                   retry_on_conflict\u003dFalse)"},{"line_number":401,"context_line":"        except ironic.exc.BadRequest:"},{"line_number":402,"context_line":"            msg \u003d (_(\"Failed to add deploy parameters on node %(node)s \""},{"line_number":403,"context_line":"                     \"when provisioning the instance %(instance)s\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_dfd23934","line":400,"updated":"2015-12-01 16:54:53.000000000","message":"I wish I could see a test case for this?","commit_id":"342b39633c4495dd4c819489c0176e7ab2835604"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"08e106bbd60ae1e56e031ef03dfc120832a235ec","unresolved":false,"context_lines":[{"line_number":397,"context_line":"            # node picked by the scheduler is already associated with another"},{"line_number":398,"context_line":"            # instance due bug #1341420."},{"line_number":399,"context_line":"            self.ironicclient.call(\u0027node.update\u0027, node.uuid, patch,"},{"line_number":400,"context_line":"                                   retry_on_conflict\u003dFalse)"},{"line_number":401,"context_line":"        except ironic.exc.BadRequest:"},{"line_number":402,"context_line":"            msg \u003d (_(\"Failed to add deploy parameters on node %(node)s \""},{"line_number":403,"context_line":"                     \"when provisioning the instance %(instance)s\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_a86ffdfa","line":400,"in_reply_to":"9a8ffd7b_dfd23934","updated":"2015-12-01 17:38:20.000000000","message":"ops, it\u0027s on the way!","commit_id":"342b39633c4495dd4c819489c0176e7ab2835604"}]}