)]}'
{"nova/compute/api.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"44a19cac4fd3b9c8f1a90ad5b5f6849a67fea3fa","unresolved":false,"context_lines":[{"line_number":3849,"context_line":"            flavor\u003dnew_instance_type,"},{"line_number":3850,"context_line":"            clean_shutdown\u003dclean_shutdown,"},{"line_number":3851,"context_line":"            request_spec\u003drequest_spec,"},{"line_number":3852,"context_line":"            do_cast\u003dallow_cross_cell_resize)"},{"line_number":3853,"context_line":""},{"line_number":3854,"context_line":"    @check_instance_lock"},{"line_number":3855,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.ACTIVE, vm_states.STOPPED,"}],"source_content_type":"text/x-python","patch_set":53,"id":"3fa7e38b_b0661f55","line":3852,"range":{"start_line":3852,"start_character":12,"end_line":3852,"end_character":43},"updated":"2019-11-11 16:07:05.000000000","message":"Are you planning to add a microversion for this, once it\u0027s possible for this to be true? Right now, it\u0027s synchronous until the host is verified it sounds like, so making it async in some situations would be a bit of a change in behavior. Also, it looks like this would make any user who can do cross-cell migrations have async resizes always, which isn\u0027t necessarily a thing they would know about. Wouldn\u0027t it be confusing to be developing an app with your credentials, always get a 202 and instant return, then deploy that app with some service credentials only to find that it is getting 200s and blocking calls?","commit_id":"a2e53cacd3f6df268d861e2db69bfb0fe508419c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"74994f08681ba6570eadadf65e84b10cf80f1e93","unresolved":false,"context_lines":[{"line_number":3849,"context_line":"            flavor\u003dnew_instance_type,"},{"line_number":3850,"context_line":"            clean_shutdown\u003dclean_shutdown,"},{"line_number":3851,"context_line":"            request_spec\u003drequest_spec,"},{"line_number":3852,"context_line":"            do_cast\u003dallow_cross_cell_resize)"},{"line_number":3853,"context_line":""},{"line_number":3854,"context_line":"    @check_instance_lock"},{"line_number":3855,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.ACTIVE, vm_states.STOPPED,"}],"source_content_type":"text/x-python","patch_set":53,"id":"3fa7e38b_ecfa83b8","line":3852,"range":{"start_line":3852,"start_character":12,"end_line":3852,"end_character":43},"in_reply_to":"3fa7e38b_0b2b4833","updated":"2019-11-11 20:27:47.000000000","message":"\u003e Are you planning to add a microversion for this, once it\u0027s possible for this to be true?\n\nNo, I\u0027m not planning on that.\n\nIt\u0027s a 202 either way so the user either gets a quickish response (same cell resize assuming scheduling doesn\u0027t take a noticeable amount of time) or a quick response (cross-cell resize), but they don\u0027t know either way so I don\u0027t see the reason for a microversion.\n\nI called this out specifically in the spec and there appeared to not be issues with it, but maybe the dots weren\u0027t connecting at that time (#2 here):\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/train/approved/cross-cell-resize.html#api\n\nI don\u0027t really want to go the way of the 2.34 microversion which made live migration a cast from the API because as noted, users shouldn\u0027t have to know if they are resizing across cells and opt into it - operators want to enable it to migrate users without their knowledge really to drain old cells.\n\n\u003e It is probably an unfortunate loss of data to always make this a cast, but I think the consistency argument is a win over whatever you might glean from the synchronous call.\n\nWhat loss of data? Do you just mean immediate failures like NoValidHost during scheduling? The user still gets that stuff via instance actions if the operation fails.\n\nThe tl;dr user experience on this is:\n\n1. make the request,\n2. get a 202,\n3. poll until the server status is VERIFY_RESIZE or timeout,\n4. if timeout, check instance actions\n\nThat doesn\u0027t change whether we cast or call here. The only difference is what might fail in between and come back out of the API, but that\u0027s not really a contractual thing people should be building apps around, right? That\u0027s why we have an instance actions API for tracking the progress of an operation.","commit_id":"a2e53cacd3f6df268d861e2db69bfb0fe508419c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2152bef8eef24e869a10e4720fd00d5f44269722","unresolved":false,"context_lines":[{"line_number":3849,"context_line":"            flavor\u003dnew_instance_type,"},{"line_number":3850,"context_line":"            clean_shutdown\u003dclean_shutdown,"},{"line_number":3851,"context_line":"            request_spec\u003drequest_spec,"},{"line_number":3852,"context_line":"            do_cast\u003dallow_cross_cell_resize)"},{"line_number":3853,"context_line":""},{"line_number":3854,"context_line":"    @check_instance_lock"},{"line_number":3855,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.ACTIVE, vm_states.STOPPED,"}],"source_content_type":"text/x-python","patch_set":53,"id":"3fa7e38b_8f3421ae","line":3852,"range":{"start_line":3852,"start_character":12,"end_line":3852,"end_character":43},"in_reply_to":"3fa7e38b_4f54e9fe","updated":"2019-11-11 23:01:53.000000000","message":"Okay, since this doesn\u0027t enable anything, if you\u0027re going to switch it to always cast and update the error handling, I\u0027ll +W this and you can queue up that change. It only needs to come before the switch to turn this on, but ideally earlier for context.\n\nI imagine gibi will be okay with that change, but if not, we can argue it out on that patch at that time.","commit_id":"a2e53cacd3f6df268d861e2db69bfb0fe508419c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3002bba9f084eb51413bf11351a63fa127d6c4ab","unresolved":false,"context_lines":[{"line_number":3849,"context_line":"            flavor\u003dnew_instance_type,"},{"line_number":3850,"context_line":"            clean_shutdown\u003dclean_shutdown,"},{"line_number":3851,"context_line":"            request_spec\u003drequest_spec,"},{"line_number":3852,"context_line":"            do_cast\u003dallow_cross_cell_resize)"},{"line_number":3853,"context_line":""},{"line_number":3854,"context_line":"    @check_instance_lock"},{"line_number":3855,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.ACTIVE, vm_states.STOPPED,"}],"source_content_type":"text/x-python","patch_set":53,"id":"3fa7e38b_cf19b97a","line":3852,"range":{"start_line":3852,"start_character":12,"end_line":3852,"end_character":43},"in_reply_to":"3fa7e38b_6c353345","updated":"2019-11-11 22:28:39.000000000","message":"\u003e Okay, I thought based on the comment that it would be a 200 now,\n \u003e and maybe assumed that you\u0027d be in verify state after that if\n \u003e everything went well.\n\nAck. FWIW, the comment doesn\u0027t mention the response code but the commit message does. Since the thing that controls the response code is separate from this code I didn\u0027t mention it in the comment here.\n\n \u003e \u003e I don\u0027t really want to go the way of the 2.34 microversion which\n \u003e \u003e made live migration a cast from the API because as noted, users\n \u003e \u003e shouldn\u0027t have to know if they are resizing across cells and opt\n \u003e \u003e into it - operators want to enable it to migrate users without\n \u003e \u003e their knowledge really to drain old cells.\n \u003e \n \u003e Right, as mentioned, I don\u0027t want a microversion for this because\n \u003e it doesn\u0027t seem like something the user should opt-in to. What I do\n \u003e want is consistency. If it\u0027s 202 right now, and the user shouldn\u0027t\n \u003e be depending on the timing of the response to know if scheduling\n \u003e worked or not, why can\u0027t we just always make this a cast so that\n \u003e we\u0027re behaving the same for both cross- and same-cell migrations?\n\nWe can make it a cast, that\u0027s fine with me. It wasn\u0027t until I started work on this feature that I even realized same-cell resize was a synchronous call all the way through to the prep_resize cast to the first selected dest compute. That was a surprise to me since most API operations should cast to conductor/compute so you get a quick response.\nIf I make it a cast in all cases:\n\n1. There is likely error handling in the route handler code that could go away, like NoValidHost - I don\u0027t want to clean that up here in this patch so I\u0027m assuming people would be fine with doing that cleanup either in a follow up or creating a separate patch before this one that makes all resizes an async cast from API to conductor.\n2. Is a release note necessary for the behavior change in the same-cell resize case?\n\n \u003e \u003e The tl;dr user experience on this is:\n \u003e \u003e\n \u003e \u003e 1. make the request,\n \u003e \u003e 2. get a 202,\n \u003e \u003e 3. poll until the server status is VERIFY_RESIZE or timeout,\n \u003e \u003e 4. if timeout, check instance actions\n \u003e \u003e\n \u003e \u003e That doesn\u0027t change whether we cast or call here. The only\n \u003e \u003e difference is what might fail in between and come back out of the\n \u003e \u003e API, but that\u0027s not really a contractual thing people should be\n \u003e \u003e building apps around, right? That\u0027s why we have an instance\n \u003e actions\n \u003e \u003e API for tracking the progress of an operation.\n \u003e \n \u003e Well, I dunno if they do or not. What happens if we get a\n \u003e NoValidHost in the synchronous call? Do we still return 202 or some\n \u003e 4xx?\n\nThe user gets a 400 in that case:\n\nhttps://github.com/openstack/nova/blob/17b5a1ab85c358a1096788371849f0cccfe84972/nova/api/openstack/compute/servers.py#L978\n\n \u003e If the former, then cool, and we should be able to make them\n \u003e both cast and unify the behavior. If the latter, then this is\n \u003e indeed a change in behavior for cross-cell, in which case the\n \u003e behavior (defined or determined) will be different depending on the\n \u003e policy. Regardless of the failure case, I think it\u0027s legit to move\n \u003e them both to cast at this point, since the current 202 should mean\n \u003e you have to poll for status anyway and to avoid people building\n \u003e future reliance on the scheduling being part of the return status\n \u003e right?\n\nAgain I\u0027m OK with changing to a cast in all cases, but figured others wouldn\u0027t be so that\u0027s why I made this conditional.\n\nIt would be nice if at least gibi, who is +2 on this right now, is also OK with making this a cast in all cases so I don\u0027t have to flip flop on this later when I separate out that change and rebase the entire stack.","commit_id":"a2e53cacd3f6df268d861e2db69bfb0fe508419c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6a374ce3eba3e81de32f7c562b89a27679fcd6b6","unresolved":false,"context_lines":[{"line_number":3849,"context_line":"            flavor\u003dnew_instance_type,"},{"line_number":3850,"context_line":"            clean_shutdown\u003dclean_shutdown,"},{"line_number":3851,"context_line":"            request_spec\u003drequest_spec,"},{"line_number":3852,"context_line":"            do_cast\u003dallow_cross_cell_resize)"},{"line_number":3853,"context_line":""},{"line_number":3854,"context_line":"    @check_instance_lock"},{"line_number":3855,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.ACTIVE, vm_states.STOPPED,"}],"source_content_type":"text/x-python","patch_set":53,"id":"3fa7e38b_1d413ca7","line":3852,"range":{"start_line":3852,"start_character":12,"end_line":3852,"end_character":43},"in_reply_to":"3fa7e38b_8f3421ae","updated":"2019-11-12 21:17:33.000000000","message":"\u003e Okay, since this doesn\u0027t enable anything, if you\u0027re going to switch\n \u003e it to always cast and update the error handling, I\u0027ll +W this and\n \u003e you can queue up that change. It only needs to come before the\n \u003e switch to turn this on, but ideally earlier for context.\n \u003e \n \u003e I imagine gibi will be okay with that change, but if not, we can\n \u003e argue it out on that patch at that time.\n\nDone: https://review.opendev.org/693937","commit_id":"a2e53cacd3f6df268d861e2db69bfb0fe508419c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"233399b743a122a77fa0459b53690f8c90bbc86c","unresolved":false,"context_lines":[{"line_number":3849,"context_line":"            flavor\u003dnew_instance_type,"},{"line_number":3850,"context_line":"            clean_shutdown\u003dclean_shutdown,"},{"line_number":3851,"context_line":"            request_spec\u003drequest_spec,"},{"line_number":3852,"context_line":"            do_cast\u003dallow_cross_cell_resize)"},{"line_number":3853,"context_line":""},{"line_number":3854,"context_line":"    @check_instance_lock"},{"line_number":3855,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.ACTIVE, vm_states.STOPPED,"}],"source_content_type":"text/x-python","patch_set":53,"id":"3fa7e38b_0b2b4833","line":3852,"range":{"start_line":3852,"start_character":12,"end_line":3852,"end_character":43},"in_reply_to":"3fa7e38b_ab3a34c1","updated":"2019-11-11 16:56:19.000000000","message":"Yeah, I\u0027d prefer consistency as the user doesn\u0027t know if they\u0027re allowed to do cross-cell, whether or not they\u0027re going to get one, or even if the cloud they\u0027re on *has* multiple cells. It is probably an unfortunate loss of data to always make this a cast, but I think the consistency argument is a win over whatever you might glean from the synchronous call.\n\nI\u0027m still not sure on the microversion part. Obviously we *can* make cross-cell migration a microversion and refuse to do it for older ones (and thus keep this synchronous in those cases). However, i thought that we were never supposed to add microversions for things the user wouldn\u0027t opt-in to? Like, users won\u0027t opt-in to \"a better error message\" or \"a response body with more consistent capital letters.\" Since they don\u0027t know about cells, whether or not they have that permission, etc, why would the user choose to use microversion 2.X in this case? Obviously if 2.Y has some feature they want then they will opt-in to it over time, but...\n\nAnyway, the microversion part is something we need to decide and do, but my concern is the user-invisible inconsistent result and behavior.","commit_id":"a2e53cacd3f6df268d861e2db69bfb0fe508419c"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"45a32ea38e6765e79be7d5853dd525fbe62e8c20","unresolved":false,"context_lines":[{"line_number":3849,"context_line":"            flavor\u003dnew_instance_type,"},{"line_number":3850,"context_line":"            clean_shutdown\u003dclean_shutdown,"},{"line_number":3851,"context_line":"            request_spec\u003drequest_spec,"},{"line_number":3852,"context_line":"            do_cast\u003dallow_cross_cell_resize)"},{"line_number":3853,"context_line":""},{"line_number":3854,"context_line":"    @check_instance_lock"},{"line_number":3855,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.ACTIVE, vm_states.STOPPED,"}],"source_content_type":"text/x-python","patch_set":53,"id":"3fa7e38b_ab3a34c1","line":3852,"range":{"start_line":3852,"start_character":12,"end_line":3852,"end_character":43},"in_reply_to":"3fa7e38b_b0661f55","updated":"2019-11-11 16:38:29.000000000","message":"This is indeed a bit weird for the reasons Dan states.\n\nIf at time A user X and Y would both get the same response when doing this operation, and then at B they _might_ a get different response because of the addition of this code and a change in policy, and the response is not a 403 but some form of success (i.e., 2xx) then a microversion is warranted because code expectations are violated.\n\nThat\u0027s all further complicated by the fact that there are plenty of things happening under the covers here that the API user isn\u0027t necessarily going to be aware of (cells, policies, whatevers).\n\nIs there a way to make it always a cast from microversion X onwards? For sake of consistency?","commit_id":"a2e53cacd3f6df268d861e2db69bfb0fe508419c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7e7b2a5060690898578ee7e94ab7b6997370295f","unresolved":false,"context_lines":[{"line_number":3849,"context_line":"            flavor\u003dnew_instance_type,"},{"line_number":3850,"context_line":"            clean_shutdown\u003dclean_shutdown,"},{"line_number":3851,"context_line":"            request_spec\u003drequest_spec,"},{"line_number":3852,"context_line":"            do_cast\u003dallow_cross_cell_resize)"},{"line_number":3853,"context_line":""},{"line_number":3854,"context_line":"    @check_instance_lock"},{"line_number":3855,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.ACTIVE, vm_states.STOPPED,"}],"source_content_type":"text/x-python","patch_set":53,"id":"3fa7e38b_cf3219f3","line":3852,"range":{"start_line":3852,"start_character":12,"end_line":3852,"end_character":43},"in_reply_to":"3fa7e38b_cf19b97a","updated":"2019-11-11 22:31:32.000000000","message":"Also note that if we make this a cast in all cases then I will also be reverting this change we just recently merged:\n\nI9115ef6df59844cd6e702f19ba38ffbf9f8b35d3\n\nNot a big deal to me, just FYI.","commit_id":"a2e53cacd3f6df268d861e2db69bfb0fe508419c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ae3e69ab131a841a0e2d9166dfbd2241dd90d113","unresolved":false,"context_lines":[{"line_number":3849,"context_line":"            flavor\u003dnew_instance_type,"},{"line_number":3850,"context_line":"            clean_shutdown\u003dclean_shutdown,"},{"line_number":3851,"context_line":"            request_spec\u003drequest_spec,"},{"line_number":3852,"context_line":"            do_cast\u003dallow_cross_cell_resize)"},{"line_number":3853,"context_line":""},{"line_number":3854,"context_line":"    @check_instance_lock"},{"line_number":3855,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.ACTIVE, vm_states.STOPPED,"}],"source_content_type":"text/x-python","patch_set":53,"id":"3fa7e38b_4f54e9fe","line":3852,"range":{"start_line":3852,"start_character":12,"end_line":3852,"end_character":43},"in_reply_to":"3fa7e38b_cf3219f3","updated":"2019-11-11 22:33:37.000000000","message":"\u003e Also note that if we make this a cast in all cases then I will also\n \u003e be reverting this change we just recently merged:\n \u003e \n \u003e I9115ef6df59844cd6e702f19ba38ffbf9f8b35d3\n \u003e \n \u003e Not a big deal to me, just FYI.\n\nWell, maybe not. migrate_server is still called from the compute during a reschedule within a same-cell resize so I think I might want to leave that as a synchronous call still since otherwise it\u0027s a bigger change to deal with in the prep_resize reschedule code. So the change would just be API -\u003e (super) conductor \u003d RPC cast, leave compute -\u003e (cell) conductor \u003d RPC call.","commit_id":"a2e53cacd3f6df268d861e2db69bfb0fe508419c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a9ae6cce95b210e0b4be9a78c89ed53c152adae5","unresolved":false,"context_lines":[{"line_number":3849,"context_line":"            flavor\u003dnew_instance_type,"},{"line_number":3850,"context_line":"            clean_shutdown\u003dclean_shutdown,"},{"line_number":3851,"context_line":"            request_spec\u003drequest_spec,"},{"line_number":3852,"context_line":"            do_cast\u003dallow_cross_cell_resize)"},{"line_number":3853,"context_line":""},{"line_number":3854,"context_line":"    @check_instance_lock"},{"line_number":3855,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.ACTIVE, vm_states.STOPPED,"}],"source_content_type":"text/x-python","patch_set":53,"id":"3fa7e38b_6c353345","line":3852,"range":{"start_line":3852,"start_character":12,"end_line":3852,"end_character":43},"in_reply_to":"3fa7e38b_ecfa83b8","updated":"2019-11-11 21:15:28.000000000","message":"\u003e It\u0027s a 202 either way so the user either gets a quickish response\n \u003e (same cell resize assuming scheduling doesn\u0027t take a noticeable\n \u003e amount of time) or a quick response (cross-cell resize), but they\n \u003e don\u0027t know either way so I don\u0027t see the reason for a microversion.\n\nOkay, I thought based on the comment that it would be a 200 now, and maybe assumed that you\u0027d be in verify state after that if everything went well.\n\n \u003e I called this out specifically in the spec and there appeared to\n \u003e not be issues with it, but maybe the dots weren\u0027t connecting at\n \u003e that time (#2 here):\n\nYeah, forgive me for not being able to get this all in my head ahead of time.\n\n \u003e I don\u0027t really want to go the way of the 2.34 microversion which\n \u003e made live migration a cast from the API because as noted, users\n \u003e shouldn\u0027t have to know if they are resizing across cells and opt\n \u003e into it - operators want to enable it to migrate users without\n \u003e their knowledge really to drain old cells.\n\nRight, as mentioned, I don\u0027t want a microversion for this because it doesn\u0027t seem like something the user should opt-in to. What I do want is consistency. If it\u0027s 202 right now, and the user shouldn\u0027t be depending on the timing of the response to know if scheduling worked or not, why can\u0027t we just always make this a cast so that we\u0027re behaving the same for both cross- and same-cell migrations?\n\n \u003e What loss of data? Do you just mean immediate failures like\n \u003e NoValidHost during scheduling? The user still gets that stuff via\n \u003e instance actions if the operation fails.\n\nYes, that\u0027s what I meant, but see above.\n\n \u003e The tl;dr user experience on this is:\n \u003e \n \u003e 1. make the request,\n \u003e 2. get a 202,\n \u003e 3. poll until the server status is VERIFY_RESIZE or timeout,\n \u003e 4. if timeout, check instance actions\n \u003e \n \u003e That doesn\u0027t change whether we cast or call here. The only\n \u003e difference is what might fail in between and come back out of the\n \u003e API, but that\u0027s not really a contractual thing people should be\n \u003e building apps around, right? That\u0027s why we have an instance actions\n \u003e API for tracking the progress of an operation.\n\nWell, I dunno if they do or not. What happens if we get a NoValidHost in the synchronous call? Do we still return 202 or some 4xx? If the former, then cool, and we should be able to make them both cast and unify the behavior. If the latter, then this is indeed a change in behavior for cross-cell, in which case the behavior (defined or determined) will be different depending on the policy. Regardless of the failure case, I think it\u0027s legit to move them both to cast at this point, since the current 202 should mean you have to poll for status anyway and to avoid people building future reliance on the scheduling being part of the return status right?","commit_id":"a2e53cacd3f6df268d861e2db69bfb0fe508419c"}],"nova/conductor/api.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a630b05bbad1e01a67f161a3ef38aaa57750c9d5","unresolved":false,"context_lines":[{"line_number":89,"context_line":"    def resize_instance(self, context, instance, scheduler_hint, flavor,"},{"line_number":90,"context_line":"                        reservations\u003dNone, clean_shutdown\u003dTrue,"},{"line_number":91,"context_line":"                        request_spec\u003dNone, host_list\u003dNone, do_cast\u003dFalse):"},{"line_number":92,"context_line":"        # NOTE(comstud): \u0027extra_instance_updates\u0027 is not used here but is"},{"line_number":93,"context_line":"        # needed for compatibility with the cells_rpcapi version of this"},{"line_number":94,"context_line":"        # method."},{"line_number":95,"context_line":"        self.conductor_compute_rpcapi.migrate_server("}],"source_content_type":"text/x-python","patch_set":41,"id":"7faddb67_a3c0b451","line":92,"updated":"2019-08-16 10:23:56.000000000","message":"Is it a rebase artifact that this comment appears in this patch? feels unrelated.","commit_id":"e2d945c82447f9c604f1bceca654cc2aaad5362f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"74df28d7281c3528676333fe85eb04d900caaeee","unresolved":false,"context_lines":[{"line_number":89,"context_line":"    def resize_instance(self, context, instance, scheduler_hint, flavor,"},{"line_number":90,"context_line":"                        reservations\u003dNone, clean_shutdown\u003dTrue,"},{"line_number":91,"context_line":"                        request_spec\u003dNone, host_list\u003dNone, do_cast\u003dFalse):"},{"line_number":92,"context_line":"        # NOTE(comstud): \u0027extra_instance_updates\u0027 is not used here but is"},{"line_number":93,"context_line":"        # needed for compatibility with the cells_rpcapi version of this"},{"line_number":94,"context_line":"        # method."},{"line_number":95,"context_line":"        self.conductor_compute_rpcapi.migrate_server("}],"source_content_type":"text/x-python","patch_set":41,"id":"7faddb67_f036c24f","line":92,"in_reply_to":"7faddb67_03488fb0","updated":"2019-08-27 18:56:57.000000000","message":"Done","commit_id":"e2d945c82447f9c604f1bceca654cc2aaad5362f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"27c937f91acf20a32e308668b04293677bc12bc6","unresolved":false,"context_lines":[{"line_number":89,"context_line":"    def resize_instance(self, context, instance, scheduler_hint, flavor,"},{"line_number":90,"context_line":"                        reservations\u003dNone, clean_shutdown\u003dTrue,"},{"line_number":91,"context_line":"                        request_spec\u003dNone, host_list\u003dNone, do_cast\u003dFalse):"},{"line_number":92,"context_line":"        # NOTE(comstud): \u0027extra_instance_updates\u0027 is not used here but is"},{"line_number":93,"context_line":"        # needed for compatibility with the cells_rpcapi version of this"},{"line_number":94,"context_line":"        # method."},{"line_number":95,"context_line":"        self.conductor_compute_rpcapi.migrate_server("}],"source_content_type":"text/x-python","patch_set":41,"id":"7faddb67_03488fb0","line":92,"in_reply_to":"7faddb67_a3c0b451","updated":"2019-08-19 14:15:28.000000000","message":"Yes, bad rebase from when Stephen removed the extra_instance_updates kwarg: I70012f7be863afc9d9ed8882cc5d9d193bbb7b6d","commit_id":"e2d945c82447f9c604f1bceca654cc2aaad5362f"}],"nova/conductor/rpcapi.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a630b05bbad1e01a67f161a3ef38aaa57750c9d5","unresolved":false,"context_lines":[{"line_number":336,"context_line":"            version \u003d \u00271.4\u0027"},{"line_number":337,"context_line":"        cctxt \u003d self.client.prepare(version\u003dversion)"},{"line_number":338,"context_line":"        if do_cast:"},{"line_number":339,"context_line":"            return cctxt.cast(context, \u0027migrate_server\u0027, **kw)"},{"line_number":340,"context_line":"        return cctxt.call(context, \u0027migrate_server\u0027, **kw)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    def build_instances(self, context, instances, image, filter_properties,"}],"source_content_type":"text/x-python","patch_set":41,"id":"7faddb67_e3d22cfd","line":339,"updated":"2019-08-16 10:23:56.000000000","message":"There is a list of expected exceptions decorated on the migrate_server function of the conductor.manager. Do I understand correctly that with call() those exceptions went back to the API user, but with the new cast() path, those will be dropped? By reading the doc of that decorator I\u0027m not even sure that the these exception will be logged when they bubble up to the top of the RPC method.","commit_id":"e2d945c82447f9c604f1bceca654cc2aaad5362f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"37223dea7e40981d25d9c3a5fa31c31abed95a17","unresolved":false,"context_lines":[{"line_number":336,"context_line":"            version \u003d \u00271.4\u0027"},{"line_number":337,"context_line":"        cctxt \u003d self.client.prepare(version\u003dversion)"},{"line_number":338,"context_line":"        if do_cast:"},{"line_number":339,"context_line":"            return cctxt.cast(context, \u0027migrate_server\u0027, **kw)"},{"line_number":340,"context_line":"        return cctxt.call(context, \u0027migrate_server\u0027, **kw)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    def build_instances(self, context, instances, image, filter_properties,"}],"source_content_type":"text/x-python","patch_set":41,"id":"7faddb67_102502c2","line":339,"in_reply_to":"7faddb67_3e3844d1","updated":"2019-08-21 16:15:01.000000000","message":"Thanks. \n\nSo there is a catch-all in the conductor manager that leads to a warning log (and to error state and notification sending). I rest my case.","commit_id":"e2d945c82447f9c604f1bceca654cc2aaad5362f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"27c937f91acf20a32e308668b04293677bc12bc6","unresolved":false,"context_lines":[{"line_number":336,"context_line":"            version \u003d \u00271.4\u0027"},{"line_number":337,"context_line":"        cctxt \u003d self.client.prepare(version\u003dversion)"},{"line_number":338,"context_line":"        if do_cast:"},{"line_number":339,"context_line":"            return cctxt.cast(context, \u0027migrate_server\u0027, **kw)"},{"line_number":340,"context_line":"        return cctxt.call(context, \u0027migrate_server\u0027, **kw)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    def build_instances(self, context, instances, image, filter_properties,"}],"source_content_type":"text/x-python","patch_set":41,"id":"7faddb67_3e3844d1","line":339,"in_reply_to":"7faddb67_e3d22cfd","updated":"2019-08-19 14:15:28.000000000","message":"\u003e There is a list of expected exceptions decorated on the\n \u003e migrate_server function of the conductor.manager. Do I understand\n \u003e correctly that with call() those exceptions went back to the API\n \u003e user, but with the new cast() path, those will be dropped?\n\nCorrect. With the call they\u0027d be handled in the wsgi controller code:\n\nhttps://github.com/openstack/nova/blob/ee6b69cadc75219aa53bdb91bb71a8b0cedd215b/nova/api/openstack/compute/migrate_server.py#L71\n\nhttps://github.com/openstack/nova/blob/ee6b69cadc75219aa53bdb91bb71a8b0cedd215b/nova/api/openstack/compute/servers.py#L951\n\n By\n \u003e reading the doc of that decorator I\u0027m not even sure that the these\n \u003e exception will be logged when they bubble up to the top of the RPC\n \u003e method.\n\nI think if you bubble an exception up to the expected_exceptions decorator it only gets logged (as an exception) if it\u0027s not expected:\n\nhttps://github.com/openstack/oslo.messaging/blob/40c25c2bde6d2f5a756e7169060b7ce389caf174/oslo_messaging/rpc/server.py#L169\n\nOtherwise it\u0027s logged at debug.\n\nNote that there is error handling here:\n\nhttps://github.com/openstack/nova/blob/ee6b69cadc75219aa53bdb91bb71a8b0cedd215b/nova/conductor/manager.py#L332\n\nAnd we\u0027ll log the error (and send a notification and record a fault) here:\n\nhttps://github.com/openstack/nova/blob/ee6b69cadc75219aa53bdb91bb71a8b0cedd215b/nova/scheduler/utils.py#L723","commit_id":"e2d945c82447f9c604f1bceca654cc2aaad5362f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"01e96c10d1e2b948fc2aeca729e249c19d96bf78","unresolved":false,"context_lines":[{"line_number":339,"context_line":"                    objects_base.obj_to_primitive(instance))"},{"line_number":340,"context_line":"            version \u003d \u00271.4\u0027"},{"line_number":341,"context_line":"        cctxt \u003d self.client.prepare(version\u003dversion)"},{"line_number":342,"context_line":"        if do_cast:"},{"line_number":343,"context_line":"            return cctxt.cast(context, \u0027migrate_server\u0027, **kw)"},{"line_number":344,"context_line":"        return cctxt.call(context, \u0027migrate_server\u0027, **kw)"},{"line_number":345,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"3fa7e38b_869bf8c6","line":342,"updated":"2019-11-04 17:42:03.000000000","message":"Merge conflict on this now:\n\nhttps://review.opendev.org/692550","commit_id":"004a06609fea6507c95afee5b2229e705b825d87"}]}