)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"293ffc967f5d820b4325cdcb848236f56691b0e6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"612ded0e_2a31417b","updated":"2025-12-15 12:35:36.000000000","message":"Tested with operators deployment in https://github.com/openstack-k8s-operators/watcher-operator/pull/316\n\nSee the retries are effectively working with default migration_interval of 5 seconds:\n\nhttps://logserver.rdoproject.org/ce5/rdoproject.org/ce540fa3e0ca45d2a868f94d0c6dfd3c/controller/ci-framework-data/logs/openstack-must-gather/quay-io-openstack-k8s-operators-openstack-must-gather-sha256-854a802357b4f565a366fce3bf29b20c1b768ec4ab7e822ef52dfc2fef000d2c/namespaces/openstack/pods/watcher-applier-0/logs/watcher-applier.log.gz\n\n2025-12-15 12:03:04.660 1 WARNING watcher.common.nova_helper [None req-fa072004-55c8-46c6-9112-00554d4ffc65 - - - - - -] Retrying connection to Nova service: keystoneauth1.exceptions.connection.ConnectFailure: Unable to establish connection to https://nova-internal.openstack.svc:8774/v2.1/servers/09bfcad0-1328-49b6-a9e0-51267581056e: (\u0027Connection aborted.\u0027, RemoteDisconnected(\u0027Remote end closed connection without response\u0027))","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"635678a09a8a69b1ce83f8a1ed331b0227b9e89f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"41162cea_a3542000","updated":"2025-12-15 14:39:46.000000000","message":"recheck","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"393068d5cc9d1b7fad80dcc6d9d15d6893363f43","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"64effda5_a1e70416","updated":"2025-12-17 13:42:45.000000000","message":"lgtm, thanks for the changes!","commit_id":"1d32e734f34db23b5aa444f9c3d22a13bef058b1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ec35ef942c4ac5ed2cfeffee5dac0a2f1562b76e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b63d1991_5d3605d7","updated":"2025-12-17 14:50:24.000000000","message":"this is a little less granular then i was expecting but its also less code changeae\nas a reult.\ni think htis is work about but hopefully wew can delegate some fo this to the sdk in the future.","commit_id":"1d32e734f34db23b5aa444f9c3d22a13bef058b1"}],"watcher/common/nova_helper.py":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"8fa8c71bfb68d13ba0199dd5246145b6d467ff53","unresolved":true,"context_lines":[{"line_number":39,"context_line":"    def wrapper(*args, **kwargs):"},{"line_number":40,"context_line":"        retries \u003d CONF.nova.http_retries"},{"line_number":41,"context_line":"        retry_interval \u003d CONF.nova.http_retry_interval"},{"line_number":42,"context_line":"        for i in range(retries + 1):"},{"line_number":43,"context_line":"            try:"},{"line_number":44,"context_line":"                return call(*args, **kwargs)"},{"line_number":45,"context_line":"            except ksa_exc.ConnectionError as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"03a9d24a_c30e31ed","line":42,"updated":"2025-12-16 15:58:10.000000000","message":"nit: the current loop can do end up doing one more retry than configured (it\u0027s not a huge problem, but could be slightly confusing when debugging for example). With a slight change to the `range` call it could be solved (we\u0027d also need to adjust the error log in line 52)\n\n```suggestion\n        for i in range(1, retries + 1):\n        \n```","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"23d7584762f652f775b084bcc8b53eff65c66d93","unresolved":true,"context_lines":[{"line_number":39,"context_line":"    def wrapper(*args, **kwargs):"},{"line_number":40,"context_line":"        retries \u003d CONF.nova.http_retries"},{"line_number":41,"context_line":"        retry_interval \u003d CONF.nova.http_retry_interval"},{"line_number":42,"context_line":"        for i in range(retries + 1):"},{"line_number":43,"context_line":"            try:"},{"line_number":44,"context_line":"                return call(*args, **kwargs)"},{"line_number":45,"context_line":"            except ksa_exc.ConnectionError as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"48746f78_e1814d0c","line":42,"in_reply_to":"03a9d24a_c30e31ed","updated":"2025-12-17 09:14:32.000000000","message":"I did that on purpose. For me retries is the number of sleeps + call, so if i set retries to 3, I expect 1 call before any sleep + 3 sleeps/call before failing. That\u0027s what I\u0027m asserting in the tests.","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"ca29b0865cb9f1b7aa87fd92ed1bff73c200605d","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    def wrapper(*args, **kwargs):"},{"line_number":40,"context_line":"        retries \u003d CONF.nova.http_retries"},{"line_number":41,"context_line":"        retry_interval \u003d CONF.nova.http_retry_interval"},{"line_number":42,"context_line":"        for i in range(retries + 1):"},{"line_number":43,"context_line":"            try:"},{"line_number":44,"context_line":"                return call(*args, **kwargs)"},{"line_number":45,"context_line":"            except ksa_exc.ConnectionError as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ae9aceb_e53e726b","line":42,"in_reply_to":"48746f78_e1814d0c","updated":"2025-12-17 12:02:58.000000000","message":"ok, I see your point, lgtm then","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"23d7584762f652f775b084bcc8b53eff65c66d93","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        retry_interval \u003d CONF.nova.http_retry_interval"},{"line_number":42,"context_line":"        for i in range(retries + 1):"},{"line_number":43,"context_line":"            try:"},{"line_number":44,"context_line":"                return call(*args, **kwargs)"},{"line_number":45,"context_line":"            except ksa_exc.ConnectionError as e:"},{"line_number":46,"context_line":"                LOG.warning(\u0027Error connecting to Nova service: %s\u0027, e)"},{"line_number":47,"context_line":"                if i \u003c retries:"}],"source_content_type":"text/x-python","patch_set":1,"id":"b0f7231b_50fbc6c3","line":44,"in_reply_to":"912363a8_8ae217f9","updated":"2025-12-17 09:14:32.000000000","message":"api calls have its own timeout mechanism from the underlying libraries that i don\u0027t want to interfere with.","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"23d7584762f652f775b084bcc8b53eff65c66d93","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        for i in range(retries + 1):"},{"line_number":43,"context_line":"            try:"},{"line_number":44,"context_line":"                return call(*args, **kwargs)"},{"line_number":45,"context_line":"            except ksa_exc.ConnectionError as e:"},{"line_number":46,"context_line":"                LOG.warning(\u0027Error connecting to Nova service: %s\u0027, e)"},{"line_number":47,"context_line":"                if i \u003c retries:"},{"line_number":48,"context_line":"                    LOG.warning(\u0027Retrying connection to Nova service\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ded9ea9e_47ac805f","line":45,"in_reply_to":"74e35f57_3405a072","updated":"2025-12-17 09:14:32.000000000","message":"I think this is correct, keystoneauth1 should be catching all connection related isuues  and using ConnectionError for them.","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ec35ef942c4ac5ed2cfeffee5dac0a2f1562b76e","unresolved":true,"context_lines":[{"line_number":46,"context_line":"                LOG.warning(\u0027Error connecting to Nova service: %s\u0027, e)"},{"line_number":47,"context_line":"                if i \u003c retries:"},{"line_number":48,"context_line":"                    LOG.warning(\u0027Retrying connection to Nova service\u0027)"},{"line_number":49,"context_line":"                    time.sleep(retry_interval)"},{"line_number":50,"context_line":"                else:"},{"line_number":51,"context_line":"                    LOG.error("},{"line_number":52,"context_line":"                        \u0027Failed to connect to Nova service after %d attempts\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"8a9f861f_cc54afc2","line":49,"in_reply_to":"a191d055_e5b29751","updated":"2025-12-17 14:50:24.000000000","message":"well in aggregate we might but i agree for now we do not need this.","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"23d7584762f652f775b084bcc8b53eff65c66d93","unresolved":true,"context_lines":[{"line_number":46,"context_line":"                LOG.warning(\u0027Error connecting to Nova service: %s\u0027, e)"},{"line_number":47,"context_line":"                if i \u003c retries:"},{"line_number":48,"context_line":"                    LOG.warning(\u0027Retrying connection to Nova service\u0027)"},{"line_number":49,"context_line":"                    time.sleep(retry_interval)"},{"line_number":50,"context_line":"                else:"},{"line_number":51,"context_line":"                    LOG.error("},{"line_number":52,"context_line":"                        \u0027Failed to connect to Nova service after %d attempts\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"a191d055_e5b29751","line":49,"in_reply_to":"fee0f6d7_541a7d69","updated":"2025-12-17 09:14:32.000000000","message":"no need, INMO. That would be good if I would expect multiple calls simultaneously.","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"8fa8c71bfb68d13ba0199dd5246145b6d467ff53","unresolved":true,"context_lines":[{"line_number":312,"context_line":"        flavor_id \u003d None"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        try:"},{"line_number":315,"context_line":"            flavor_id \u003d self.nova.flavors.get(flavor).id"},{"line_number":316,"context_line":"        except nvexceptions.NotFound:"},{"line_number":317,"context_line":"            flavor_id \u003d [f.id for f in self.nova.flavors.list() if"},{"line_number":318,"context_line":"                         f.name \u003d\u003d flavor][0]"}],"source_content_type":"text/x-python","patch_set":1,"id":"af03c21c_a68975ef","line":315,"updated":"2025-12-16 15:58:10.000000000","message":"we have a couple of calls to the api in this method to get and list flavors, so we should also wrap `resize_instance` or extract the calls to other methods like you did with the start and stop instance methods","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"f397e183b49eb6f717e73e99c79ce296a1f82dc0","unresolved":false,"context_lines":[{"line_number":312,"context_line":"        flavor_id \u003d None"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        try:"},{"line_number":315,"context_line":"            flavor_id \u003d self.nova.flavors.get(flavor).id"},{"line_number":316,"context_line":"        except nvexceptions.NotFound:"},{"line_number":317,"context_line":"            flavor_id \u003d [f.id for f in self.nova.flavors.list() if"},{"line_number":318,"context_line":"                         f.name \u003d\u003d flavor][0]"}],"source_content_type":"text/x-python","patch_set":1,"id":"639c3af6_6ddd1d6c","line":315,"in_reply_to":"0fc971b2_41b4b25b","updated":"2025-12-17 13:04:17.000000000","message":"Done","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"393068d5cc9d1b7fad80dcc6d9d15d6893363f43","unresolved":false,"context_lines":[{"line_number":312,"context_line":"        flavor_id \u003d None"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        try:"},{"line_number":315,"context_line":"            flavor_id \u003d self.nova.flavors.get(flavor).id"},{"line_number":316,"context_line":"        except nvexceptions.NotFound:"},{"line_number":317,"context_line":"            flavor_id \u003d [f.id for f in self.nova.flavors.list() if"},{"line_number":318,"context_line":"                         f.name \u003d\u003d flavor][0]"}],"source_content_type":"text/x-python","patch_set":1,"id":"e85f1722_63a36a08","line":315,"in_reply_to":"0fc971b2_41b4b25b","updated":"2025-12-17 13:42:45.000000000","message":"Done","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"70a6b10cc582948c7d1f74918e55ee11b834bc02","unresolved":true,"context_lines":[{"line_number":312,"context_line":"        flavor_id \u003d None"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        try:"},{"line_number":315,"context_line":"            flavor_id \u003d self.nova.flavors.get(flavor).id"},{"line_number":316,"context_line":"        except nvexceptions.NotFound:"},{"line_number":317,"context_line":"            flavor_id \u003d [f.id for f in self.nova.flavors.list() if"},{"line_number":318,"context_line":"                         f.name \u003d\u003d flavor][0]"}],"source_content_type":"text/x-python","patch_set":1,"id":"0fc971b2_41b4b25b","line":315,"in_reply_to":"170cefd5_fd8ce29d","updated":"2025-12-17 12:31:27.000000000","message":"Finally, I\u0027ve moved to their own method all single calls so that i can add retries to all and call them from the more complex methods. Please check and let me know if i missed any.\n\nThanks for your feedback.","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"ca29b0865cb9f1b7aa87fd92ed1bff73c200605d","unresolved":true,"context_lines":[{"line_number":312,"context_line":"        flavor_id \u003d None"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        try:"},{"line_number":315,"context_line":"            flavor_id \u003d self.nova.flavors.get(flavor).id"},{"line_number":316,"context_line":"        except nvexceptions.NotFound:"},{"line_number":317,"context_line":"            flavor_id \u003d [f.id for f in self.nova.flavors.list() if"},{"line_number":318,"context_line":"                         f.name \u003d\u003d flavor][0]"}],"source_content_type":"text/x-python","patch_set":1,"id":"170cefd5_fd8ce29d","line":315,"in_reply_to":"34f94a2e_51f47bc0","updated":"2025-12-17 12:02:58.000000000","message":"good point on resize_instance, I think we should extract the flavor calls in this method however, since it\u0027s just two calls","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"23d7584762f652f775b084bcc8b53eff65c66d93","unresolved":true,"context_lines":[{"line_number":312,"context_line":"        flavor_id \u003d None"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        try:"},{"line_number":315,"context_line":"            flavor_id \u003d self.nova.flavors.get(flavor).id"},{"line_number":316,"context_line":"        except nvexceptions.NotFound:"},{"line_number":317,"context_line":"            flavor_id \u003d [f.id for f in self.nova.flavors.list() if"},{"line_number":318,"context_line":"                         f.name \u003d\u003d flavor][0]"}],"source_content_type":"text/x-python","patch_set":1,"id":"34f94a2e_51f47bc0","line":315,"in_reply_to":"af03c21c_a68975ef","updated":"2025-12-17 09:14:32.000000000","message":"I didn\u0027t wrap resize_instance because it\u0027s calling find_instance which is wrapped itself, and we may end up having http_retries^2 retries befor it finishes. From one side, I think it\u0027d be better to extract this individual calls to its own method and wrap it. In the other side it may overcomplicate it needlessly as i\u0027d extend to live_migrate, etc...","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"8fa8c71bfb68d13ba0199dd5246145b6d467ff53","unresolved":true,"context_lines":[{"line_number":390,"context_line":""},{"line_number":391,"context_line":"            # From nova api version 2.25(Mitaka release), the default value of"},{"line_number":392,"context_line":"            # block_migration is None which is mapped to \u0027auto\u0027."},{"line_number":393,"context_line":"            instance.live_migrate(host\u003ddest_hostname)"},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"            instance \u003d self.find_instance(instance_id)"},{"line_number":396,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"8c479062_2f1f5826","line":393,"updated":"2025-12-16 15:58:10.000000000","message":"this call to `live_migrate` will probably internally trigger an api call to nova right? so we should also wrap this method","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"393068d5cc9d1b7fad80dcc6d9d15d6893363f43","unresolved":false,"context_lines":[{"line_number":390,"context_line":""},{"line_number":391,"context_line":"            # From nova api version 2.25(Mitaka release), the default value of"},{"line_number":392,"context_line":"            # block_migration is None which is mapped to \u0027auto\u0027."},{"line_number":393,"context_line":"            instance.live_migrate(host\u003ddest_hostname)"},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"            instance \u003d self.find_instance(instance_id)"},{"line_number":396,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"382e25b7_eb819287","line":393,"in_reply_to":"8c479062_2f1f5826","updated":"2025-12-17 13:42:45.000000000","message":"Done","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"8fa8c71bfb68d13ba0199dd5246145b6d467ff53","unresolved":true,"context_lines":[{"line_number":443,"context_line":"        if migration:"},{"line_number":444,"context_line":"            migration_id \u003d getattr(migration[0], \"id\")"},{"line_number":445,"context_line":"            try:"},{"line_number":446,"context_line":"                self.nova.server_migrations.live_migration_abort("},{"line_number":447,"context_line":"                    server\u003dinstance_id, migration\u003dmigration_id)"},{"line_number":448,"context_line":"            except exception as e:"},{"line_number":449,"context_line":"                # Note: Does not return from here, as abort request can\u0027t be"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9993dd_6c927112","line":446,"updated":"2025-12-16 15:58:10.000000000","message":"same, we should also add retries here","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"393068d5cc9d1b7fad80dcc6d9d15d6893363f43","unresolved":false,"context_lines":[{"line_number":443,"context_line":"        if migration:"},{"line_number":444,"context_line":"            migration_id \u003d getattr(migration[0], \"id\")"},{"line_number":445,"context_line":"            try:"},{"line_number":446,"context_line":"                self.nova.server_migrations.live_migration_abort("},{"line_number":447,"context_line":"                    server\u003dinstance_id, migration\u003dmigration_id)"},{"line_number":448,"context_line":"            except exception as e:"},{"line_number":449,"context_line":"                # Note: Does not return from here, as abort request can\u0027t be"}],"source_content_type":"text/x-python","patch_set":1,"id":"00969a44_2321b9be","line":446,"in_reply_to":"da9993dd_6c927112","updated":"2025-12-17 13:42:45.000000000","message":"Done","commit_id":"61aa5aee66ef83391c68f583f0fafec8c830cfbc"}]}