)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"83435a4106eeea4bf20f718add7ac71ee2e601e7","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add a way to exit early from a wait_for_instance_event()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In some situations, we may enter into a race-safe wait context for an"},{"line_number":10,"context_line":"external event and then determine within the context that the event"},{"line_number":11,"context_line":"would have already fired or for some other reason waiting is not required."},{"line_number":12,"context_line":"In that case, this introduces a VirtAPI method to abort the wait for"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3fa7e38b_1fbe5400","line":9,"updated":"2019-11-26 19:59:08.000000000","message":"Note to self:\n\nhttps://review.opendev.org/#/c/631244/46/nova/compute/manager.py@2627","commit_id":"9fae8e2eadc770bc59a71ef728df7b980458c257"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c70cb8914e1fb34dd86c716648953fd015269142","unresolved":false,"context_lines":[{"line_number":27,"context_line":"Once we select a host in the conductor for an instance build, we can call"},{"line_number":28,"context_line":"to cyborg to start the preparation (i.e. programming) of the device on the"},{"line_number":29,"context_line":"destination host. This takes time, and by kicking it off early, we can"},{"line_number":30,"context_line":"overlap that process with other work we have to do. Because that will send"},{"line_number":31,"context_line":"an event upon completion, which will be routed to the compute, we will race"},{"line_number":32,"context_line":"to start the build process on the compute and begin the race-safe event"},{"line_number":33,"context_line":"wait in the compute manager. Instead of collapsing the window and waiting"},{"line_number":34,"context_line":"synchronously, we can trigger it early, then start the wait in the compute"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3fa7e38b_cac0aa4d","line":31,"range":{"start_line":30,"start_character":52,"end_line":31,"end_character":61},"updated":"2019-12-03 16:35:38.000000000","message":"The event only gets routed to the compute if instance.host is set and that only happens upon a successful instance_claim during the build process:\n\nhttps://github.com/openstack/nova/blob/757fc03b78d542e7262343b65eacea02ce11dd04/nova/compute/resource_tracker.py#L170\n\nSo if by chance the event comes before the instance.host is set the compute won\u0027t get the event, but that should be OK because it sounds like the compute code is going to call back to cyborg to ask if it was already done - find that it is (otherwise it wouldn\u0027t have sent the event) - and just proceed.","commit_id":"56d3cd7aa7f4d3be01fd2a5c10903fb548c49458"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"91f9c42818ac880866992e196951823bbbc59386","unresolved":false,"context_lines":[{"line_number":27,"context_line":"Once we select a host in the conductor for an instance build, we can call"},{"line_number":28,"context_line":"to cyborg to start the preparation (i.e. programming) of the device on the"},{"line_number":29,"context_line":"destination host. This takes time, and by kicking it off early, we can"},{"line_number":30,"context_line":"overlap that process with other work we have to do. Because that will send"},{"line_number":31,"context_line":"an event upon completion, which will be routed to the compute, we will race"},{"line_number":32,"context_line":"to start the build process on the compute and begin the race-safe event"},{"line_number":33,"context_line":"wait in the compute manager. Instead of collapsing the window and waiting"},{"line_number":34,"context_line":"synchronously, we can trigger it early, then start the wait in the compute"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3fa7e38b_ea8486b4","line":31,"range":{"start_line":30,"start_character":52,"end_line":31,"end_character":61},"in_reply_to":"3fa7e38b_cac0aa4d","updated":"2019-12-03 16:49:27.000000000","message":"That\u0027s correct, and we discussed this on the cyborg patch, specifically about what the 422 should mean to cyborg if indeed it sends the event before scheduling and host claim happens.","commit_id":"56d3cd7aa7f4d3be01fd2a5c10903fb548c49458"}],"nova/compute/manager.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"37ba3586ddeeb7dcb8642fb5decd416da6433a17","unresolved":false,"context_lines":[{"line_number":426,"context_line":"        self._exit_early_exc \u003d ExitEarly"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"    def exit_wait_early(self, events):"},{"line_number":429,"context_line":"        \"\"\"Exit a wait_for_instance_event() immediately and avoid"},{"line_number":430,"context_line":"        waiting for some events."},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        :param: events: A list of (name, tag) tuples for events that we should"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_2d7f7c17","line":429,"range":{"start_line":429,"start_character":11,"end_line":429,"end_character":55},"updated":"2019-11-25 23:48:58.000000000","message":"oh, here ✔","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b0d5cc4b5a962c1bac5059c43ae132def580a3c4","unresolved":false,"context_lines":[{"line_number":429,"context_line":"        \"\"\"Exit a wait_for_instance_event() immediately and avoid"},{"line_number":430,"context_line":"        waiting for some events."},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        :param: events: A list of (name, tag) tuples for events that we should"},{"line_number":433,"context_line":"                        skip waiting for during a wait_for_instance_event()."},{"line_number":434,"context_line":"        \"\"\""},{"line_number":435,"context_line":"        raise self._exit_early_exc(events\u003devents)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_d287635a","line":432,"range":{"start_line":432,"start_character":26,"end_line":432,"end_character":30},"updated":"2019-11-25 22:04:00.000000000","message":"Can this be a set instead? If nothing else, it would make L505 more efficient.\n\nBut out of curiosity, why are we bothering to allow this? Is it in case there are nested wait_for_instance_event contexts?","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2213333a9bb2e3ff0c9c5f1c67acc56850d003dd","unresolved":false,"context_lines":[{"line_number":429,"context_line":"        \"\"\"Exit a wait_for_instance_event() immediately and avoid"},{"line_number":430,"context_line":"        waiting for some events."},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        :param: events: A list of (name, tag) tuples for events that we should"},{"line_number":433,"context_line":"                        skip waiting for during a wait_for_instance_event()."},{"line_number":434,"context_line":"        \"\"\""},{"line_number":435,"context_line":"        raise self._exit_early_exc(events\u003devents)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_ce149444","line":432,"range":{"start_line":432,"start_character":26,"end_line":432,"end_character":30},"in_reply_to":"3fa7e38b_320fd739","updated":"2019-11-26 17:06:48.000000000","message":"Since this is just passing through, I\u0027ve added the setification below where we process the result (slightly differently in the next PS for simplicity).","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"473e05d4bbfe287fd93599c2317bb7a65a75bfe0","unresolved":false,"context_lines":[{"line_number":429,"context_line":"        \"\"\"Exit a wait_for_instance_event() immediately and avoid"},{"line_number":430,"context_line":"        waiting for some events."},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        :param: events: A list of (name, tag) tuples for events that we should"},{"line_number":433,"context_line":"                        skip waiting for during a wait_for_instance_event()."},{"line_number":434,"context_line":"        \"\"\""},{"line_number":435,"context_line":"        raise self._exit_early_exc(events\u003devents)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_320fd739","line":432,"range":{"start_line":432,"start_character":26,"end_line":432,"end_character":30},"in_reply_to":"3fa7e38b_92648b4a","updated":"2019-11-25 22:55:12.000000000","message":"Sorry, to clarify: it needs to be a *list-like* thing to handle multiple events and test for membership. It *can* be a set if it matters to you, it just doesn\u0027t to the code.","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"977eff52fcf87241bc7e1d4b2cc56ee2d4b1978a","unresolved":false,"context_lines":[{"line_number":429,"context_line":"        \"\"\"Exit a wait_for_instance_event() immediately and avoid"},{"line_number":430,"context_line":"        waiting for some events."},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        :param: events: A list of (name, tag) tuples for events that we should"},{"line_number":433,"context_line":"                        skip waiting for during a wait_for_instance_event()."},{"line_number":434,"context_line":"        \"\"\""},{"line_number":435,"context_line":"        raise self._exit_early_exc(events\u003devents)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_92648b4a","line":432,"range":{"start_line":432,"start_character":26,"end_line":432,"end_character":30},"in_reply_to":"3fa7e38b_d287635a","updated":"2019-11-25 22:43:20.000000000","message":"Sure, it really needs to anything we can test for membership in. We could set()ify it here if we wanted to to strip dupes, but it doesn\u0027t matter.\n\nIt needs to be a list because the whole thing is multi-event based. If you have three neutron ports, you need to listen for all three at once. If you poll and find one is already wired up but the other two are not, you want to bail and say \"don\u0027t worry about port2 but have port1 and port3 still wait.","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b0d5cc4b5a962c1bac5059c43ae132def580a3c4","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                # should all be canceled and fired immediately below,"},{"line_number":494,"context_line":"                # but don\u0027t stick around if not."},{"line_number":495,"context_line":"                deadline \u003d 0"},{"line_number":496,"context_line":"        try:"},{"line_number":497,"context_line":"            yield"},{"line_number":498,"context_line":"        except self._exit_early_exc as e:"},{"line_number":499,"context_line":"            early_events \u003d e.events"},{"line_number":500,"context_line":"        else:"},{"line_number":501,"context_line":"            early_events \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_9286cb64","line":498,"range":{"start_line":496,"start_character":0,"end_line":498,"end_character":41},"updated":"2019-11-25 22:04:00.000000000","message":"Doesn\u0027t this mean that, if you call exit_wait_early(), we\u0027ll *skip* everything after that in your context manager? That very well may be a good thing and what the caller wants, but it should definitely be called out in the docstring.\n\nOtherwise (if we don\u0027t want to do that) could we `yield` again after L499 (which should probably instead be early_events.extend(e.events))? (but also see above about making it a set)","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"37ba3586ddeeb7dcb8642fb5decd416da6433a17","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                # should all be canceled and fired immediately below,"},{"line_number":494,"context_line":"                # but don\u0027t stick around if not."},{"line_number":495,"context_line":"                deadline \u003d 0"},{"line_number":496,"context_line":"        try:"},{"line_number":497,"context_line":"            yield"},{"line_number":498,"context_line":"        except self._exit_early_exc as e:"},{"line_number":499,"context_line":"            early_events \u003d e.events"},{"line_number":500,"context_line":"        else:"},{"line_number":501,"context_line":"            early_events \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_8d91d048","line":498,"range":{"start_line":496,"start_character":0,"end_line":498,"end_character":41},"in_reply_to":"3fa7e38b_72c6af18","updated":"2019-11-25 23:48:58.000000000","message":"\u003e The docstring for the method says \"exit immediately\". I could had\n \u003e \"no seriously, we mean it\" if you want.\n\nSorry, I see it now on L429, I was looking at the blurb on L464-9, which is a bit vague (\"upon context exit\").\n\n \u003e I don\u0027t think yielding again will get us back into the context\n\nNo, probably not, unclear what @contextmanager does to make a yield\u0027ing thing work, but it would have to translate to __enter__ and __exit__ somehow...\n\nanyway, ignore, because this is clearly how you intended it to work","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bd268456455a9a7ceee0b0030d843f411f28a1f8","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                # should all be canceled and fired immediately below,"},{"line_number":494,"context_line":"                # but don\u0027t stick around if not."},{"line_number":495,"context_line":"                deadline \u003d 0"},{"line_number":496,"context_line":"        try:"},{"line_number":497,"context_line":"            yield"},{"line_number":498,"context_line":"        except self._exit_early_exc as e:"},{"line_number":499,"context_line":"            early_events \u003d e.events"},{"line_number":500,"context_line":"        else:"},{"line_number":501,"context_line":"            early_events \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_6d2a54e7","line":498,"range":{"start_line":496,"start_character":0,"end_line":498,"end_character":41},"in_reply_to":"3fa7e38b_8d91d048","updated":"2019-11-26 00:03:29.000000000","message":"\u003e \u003e The docstring for the method says \"exit immediately\". I could had\n \u003e \u003e \"no seriously, we mean it\" if you want.\n \u003e \n \u003e Sorry, I see it now on L429, I was looking at the blurb on L464-9,\n \u003e which is a bit vague (\"upon context exit\").\n \nWill add more urgency to that block when I respin.","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"977eff52fcf87241bc7e1d4b2cc56ee2d4b1978a","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                # should all be canceled and fired immediately below,"},{"line_number":494,"context_line":"                # but don\u0027t stick around if not."},{"line_number":495,"context_line":"                deadline \u003d 0"},{"line_number":496,"context_line":"        try:"},{"line_number":497,"context_line":"            yield"},{"line_number":498,"context_line":"        except self._exit_early_exc as e:"},{"line_number":499,"context_line":"            early_events \u003d e.events"},{"line_number":500,"context_line":"        else:"},{"line_number":501,"context_line":"            early_events \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_72c6af18","line":498,"range":{"start_line":496,"start_character":0,"end_line":498,"end_character":41},"in_reply_to":"3fa7e38b_9286cb64","updated":"2019-11-25 22:43:20.000000000","message":"The docstring for the method says \"exit immediately\". I could had \"no seriously, we mean it\" if you want.\n\nI don\u0027t think yielding again will get us back into the context where we left off will it? Even still, if we wanted to be able to do that, I think we\u0027d want to use something other than an exception pattern here. However, I think we should enforce that the inner code produce a list of things it has declared finished instead of jumping back and forth to avoid any sort of interaction and to avoid pinning us into a specific implementation of this in the future.","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"06954e1fbb2216bf83e5863dcfe8b094f1208e38","unresolved":false,"context_lines":[{"line_number":468,"context_line":"        events upon context exit. Note that exit_wait_early() exits"},{"line_number":469,"context_line":"        the context immediately and should be used to signal that all"},{"line_number":470,"context_line":"        work has been completed and provide the unified list of events"},{"line_number":471,"context_line":"        that need not be waited for. Waiting will begin immediately"},{"line_number":472,"context_line":"        upon early exit as if the context was exited normally."},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"        :param instance: The instance for which an event is expected"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_21c7799b","line":471,"range":{"start_line":471,"start_character":37,"end_line":471,"end_character":55},"updated":"2019-11-26 18:35:03.000000000","message":"?\n\nWaiting... for the remaining events?","commit_id":"0a9eda254c41fefaab48ac97f5f7be860b080be2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f1c86e0a3c52f1fc282201756250995cf238af93","unresolved":false,"context_lines":[{"line_number":468,"context_line":"        events upon context exit. Note that exit_wait_early() exits"},{"line_number":469,"context_line":"        the context immediately and should be used to signal that all"},{"line_number":470,"context_line":"        work has been completed and provide the unified list of events"},{"line_number":471,"context_line":"        that need not be waited for. Waiting will begin immediately"},{"line_number":472,"context_line":"        upon early exit as if the context was exited normally."},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"        :param instance: The instance for which an event is expected"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_21b3f99c","line":471,"range":{"start_line":471,"start_character":37,"end_line":471,"end_character":55},"in_reply_to":"3fa7e38b_21c7799b","updated":"2019-11-26 18:41:02.000000000","message":"Guh, I added more words to make you happy and all that did was give you more to complain about!","commit_id":"0a9eda254c41fefaab48ac97f5f7be860b080be2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fb4cdbed731cc07c5bceec25a325b934e7b0a391","unresolved":false,"context_lines":[{"line_number":412,"context_line":"                eventlet_event.send(event)"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"class ComputeVirtAPI(virtapi.VirtAPI):"},{"line_number":416,"context_line":"    def __init__(self, compute):"},{"line_number":417,"context_line":"        super(ComputeVirtAPI, self).__init__()"},{"line_number":418,"context_line":"        self._compute \u003d compute"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_69a8cf77","line":415,"range":{"start_line":415,"start_character":21,"end_line":415,"end_character":36},"updated":"2019-11-29 11:48:47.000000000","message":"so you never extedended this \nhttps://github.com/openstack/nova/blob/master/nova/virt/virtapi.py to add exit_wait_early","commit_id":"9fae8e2eadc770bc59a71ef728df7b980458c257"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"7a03504e79d3c5c5b8d99b3bbe7ac5b3d7cb751a","unresolved":false,"context_lines":[{"line_number":412,"context_line":"                eventlet_event.send(event)"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"class ComputeVirtAPI(virtapi.VirtAPI):"},{"line_number":416,"context_line":"    def __init__(self, compute):"},{"line_number":417,"context_line":"        super(ComputeVirtAPI, self).__init__()"},{"line_number":418,"context_line":"        self._compute \u003d compute"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_e1c12431","line":415,"range":{"start_line":415,"start_character":21,"end_line":415,"end_character":36},"in_reply_to":"3fa7e38b_69a8cf77","updated":"2019-12-02 13:34:19.000000000","message":"Oh, yeah, that should be done. And FakeVirtAPI and FakeVirtAPITest.","commit_id":"9fae8e2eadc770bc59a71ef728df7b980458c257"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"313ca8c6d558873c7dd58c6b17e33810f0365c84","unresolved":false,"context_lines":[{"line_number":412,"context_line":"                eventlet_event.send(event)"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"class ComputeVirtAPI(virtapi.VirtAPI):"},{"line_number":416,"context_line":"    def __init__(self, compute):"},{"line_number":417,"context_line":"        super(ComputeVirtAPI, self).__init__()"},{"line_number":418,"context_line":"        self._compute \u003d compute"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_b6728daf","line":415,"range":{"start_line":415,"start_character":21,"end_line":415,"end_character":36},"in_reply_to":"3fa7e38b_e1c12431","updated":"2019-12-02 14:53:05.000000000","message":"Ah, that really needs to go away in a modern world.\n\nIt\u0027s from a time when compute manager was swap-out-able and so we defined the interface separate from the main compute manager implementation. It was full of documented calls for the virt driver when we were removing all db access:\n\nhttps://github.com/openstack/nova/blob/havana-eol/nova/virt/virtapi.py\n\nAt this point, there can only ever be the one implementation of it, so there\u0027s really no point in having to subclass a useless base, IMHO.\n\nI\u0027ll see about cleaning that up after this.","commit_id":"9fae8e2eadc770bc59a71ef728df7b980458c257"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"367c3fc85e4869b3904e567ab0023d4fdb11f165","unresolved":false,"context_lines":[{"line_number":423,"context_line":"                super(Exception, self).__init__()"},{"line_number":424,"context_line":"                self.events \u003d events"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"        self._exit_early_exc \u003d ExitEarly"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"    def exit_wait_early(self, events):"},{"line_number":429,"context_line":"        \"\"\"Exit a wait_for_instance_event() immediately and avoid"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_a75e074f","line":426,"updated":"2019-11-27 19:29:23.000000000","message":"So this exists (instead of just defining the ExitEarly class in exit_wait_early() and raising it directly) because we also need to catch it on L503.","commit_id":"9fae8e2eadc770bc59a71ef728df7b980458c257"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bd5ccdda8ef7644cc0d44f84eec3d0aeda421833","unresolved":false,"context_lines":[{"line_number":432,"context_line":"        :param: events: A list of (name, tag) tuples for events that we should"},{"line_number":433,"context_line":"                        skip waiting for during a wait_for_instance_event()."},{"line_number":434,"context_line":"        \"\"\""},{"line_number":435,"context_line":"        raise self._exit_early_exc(events\u003devents)"},{"line_number":436,"context_line":""},{"line_number":437,"context_line":"    def _default_error_callback(self, event_name, instance):"},{"line_number":438,"context_line":"        raise exception.NovaException(_(\u0027Instance event failed\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_6ab416d5","line":435,"range":{"start_line":435,"start_character":8,"end_line":435,"end_character":49},"updated":"2019-12-03 16:44:05.000000000","message":"im not really a huge fan of using exceptions for contol flow.\nbut that is a minor thing. it just makes following what is going on more complex.","commit_id":"56d3cd7aa7f4d3be01fd2a5c10903fb548c49458"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"91f9c42818ac880866992e196951823bbbc59386","unresolved":false,"context_lines":[{"line_number":432,"context_line":"        :param: events: A list of (name, tag) tuples for events that we should"},{"line_number":433,"context_line":"                        skip waiting for during a wait_for_instance_event()."},{"line_number":434,"context_line":"        \"\"\""},{"line_number":435,"context_line":"        raise self._exit_early_exc(events\u003devents)"},{"line_number":436,"context_line":""},{"line_number":437,"context_line":"    def _default_error_callback(self, event_name, instance):"},{"line_number":438,"context_line":"        raise exception.NovaException(_(\u0027Instance event failed\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_6a5f364d","line":435,"range":{"start_line":435,"start_character":8,"end_line":435,"end_character":49},"in_reply_to":"3fa7e38b_6ab416d5","updated":"2019-12-03 16:49:27.000000000","message":"As discussed earlier, I specifically want an exception here because I do not want you to be able to signal events to skip before you run some code. The whole point of the existing mechanism is that you prepare...do something to trigger the events...exit and wait. I did not want to be able to do some stuff, indicate some events to wait, do some more stuff, potentially indicate more events, etc.","commit_id":"56d3cd7aa7f4d3be01fd2a5c10903fb548c49458"}],"nova/tests/unit/compute/test_virtapi.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b0d5cc4b5a962c1bac5059c43ae132def580a3c4","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        self.assertRaises(test.TestingException, do_test)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def test_wait_for_instance_event_exit_early(self):"},{"line_number":186,"context_line":"        @mock.patch.object(self.virtapi._compute, \u0027_event_waiter\u0027)"},{"line_number":187,"context_line":"        def do_test(waiter):"},{"line_number":188,"context_line":"            with self.virtapi.wait_for_instance_event(\u0027instance\u0027,"},{"line_number":189,"context_line":"                                                      [(\u0027foo\u0027, \u0027bar\u0027)]):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_f2113f54","line":186,"range":{"start_line":186,"start_character":0,"end_line":186,"end_character":66},"updated":"2019-11-25 22:04:00.000000000","message":"This is kind of an odd construct; why wouldn\u0027t we use `with mock.patch.object` instead?","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"977eff52fcf87241bc7e1d4b2cc56ee2d4b1978a","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        self.assertRaises(test.TestingException, do_test)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def test_wait_for_instance_event_exit_early(self):"},{"line_number":186,"context_line":"        @mock.patch.object(self.virtapi._compute, \u0027_event_waiter\u0027)"},{"line_number":187,"context_line":"        def do_test(waiter):"},{"line_number":188,"context_line":"            with self.virtapi.wait_for_instance_event(\u0027instance\u0027,"},{"line_number":189,"context_line":"                                                      [(\u0027foo\u0027, \u0027bar\u0027)]):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_12d5fbb6","line":186,"range":{"start_line":186,"start_character":0,"end_line":186,"end_character":66},"in_reply_to":"3fa7e38b_f2113f54","updated":"2019-11-25 22:43:20.000000000","message":"What is odd? Using a closure within a test to get access to self? It\u0027s very common in our tests, and definitely here (see the several methods for testing the waiter above. In general, any time I have more than one thing to patch I prefer the closure pattern instead of the context manager because with..with..code gets hard to read. Also, python2 can\u0027t have a multi-line with statement (not sure about python3), so your context manager usage always had to be small, hence the closure pattern.","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b0d5cc4b5a962c1bac5059c43ae132def580a3c4","unresolved":false,"context_lines":[{"line_number":192,"context_line":""},{"line_number":193,"context_line":"        do_test()"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        with self.virtapi.wait_for_instance_event(\u0027instance\u0027,"},{"line_number":196,"context_line":"                                                  [(\u0027foo\u0027, \u0027bar\u0027)]):"},{"line_number":197,"context_line":"            self.virtapi.exit_wait_early([(\u0027foo\u0027, \u0027baz\u0027)])"},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_922a4b7b","line":195,"updated":"2019-11-25 22:04:00.000000000","message":"tbc, you\u0027re doing this again on purpose without mocking the waiter ... to prove that it still completes? Why would it not?\n\nIf this isn\u0027t a typo, I would suggest you take the mock decorator off of do_test and just define it as a regular method, so then you can invoke it once under a `with mock.patch.object` and once bare.","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"977eff52fcf87241bc7e1d4b2cc56ee2d4b1978a","unresolved":false,"context_lines":[{"line_number":192,"context_line":""},{"line_number":193,"context_line":"        do_test()"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        with self.virtapi.wait_for_instance_event(\u0027instance\u0027,"},{"line_number":196,"context_line":"                                                  [(\u0027foo\u0027, \u0027bar\u0027)]):"},{"line_number":197,"context_line":"            self.virtapi.exit_wait_early([(\u0027foo\u0027, \u0027baz\u0027)])"},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_32e597a4","line":195,"in_reply_to":"3fa7e38b_922a4b7b","updated":"2019-11-25 22:43:20.000000000","message":"I\u0027m doing it without mocking out the waiter which gives us the default behavior of completing the events as the regular code does. It works when mocked above because we\u0027re exiting early. If we didn\u0027t, it would raise an error about the event not being completed (see the fakes at the top of this file).","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b0d5cc4b5a962c1bac5059c43ae132def580a3c4","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        with self.virtapi.wait_for_instance_event(\u0027instance\u0027,"},{"line_number":196,"context_line":"                                                  [(\u0027foo\u0027, \u0027bar\u0027)]):"},{"line_number":197,"context_line":"            self.virtapi.exit_wait_early([(\u0027foo\u0027, \u0027baz\u0027)])"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def test_update_compute_provider_status(self):"},{"line_number":200,"context_line":"        \"\"\"Tests scenarios for adding/removing the COMPUTE_STATUS_DISABLED"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_d240c334","line":197,"updated":"2019-11-25 22:04:00.000000000","message":"Given the way the code is shaped, IMO we should have a test that waits for multiple events, calls exit_wait_early for a subset of them, and proves that we still waited for the others.","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2213333a9bb2e3ff0c9c5f1c67acc56850d003dd","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        with self.virtapi.wait_for_instance_event(\u0027instance\u0027,"},{"line_number":196,"context_line":"                                                  [(\u0027foo\u0027, \u0027bar\u0027)]):"},{"line_number":197,"context_line":"            self.virtapi.exit_wait_early([(\u0027foo\u0027, \u0027baz\u0027)])"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def test_update_compute_provider_status(self):"},{"line_number":200,"context_line":"        \"\"\"Tests scenarios for adding/removing the COMPUTE_STATUS_DISABLED"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_6e7fa069","line":197,"in_reply_to":"3fa7e38b_2d30dc54","updated":"2019-11-26 17:06:48.000000000","message":"I\u0027m changing this test to be much more straightforward and less obscure, which I think will help.","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bd268456455a9a7ceee0b0030d843f411f28a1f8","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        with self.virtapi.wait_for_instance_event(\u0027instance\u0027,"},{"line_number":196,"context_line":"                                                  [(\u0027foo\u0027, \u0027bar\u0027)]):"},{"line_number":197,"context_line":"            self.virtapi.exit_wait_early([(\u0027foo\u0027, \u0027baz\u0027)])"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def test_update_compute_provider_status(self):"},{"line_number":200,"context_line":"        \"\"\"Tests scenarios for adding/removing the COMPUTE_STATUS_DISABLED"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_2d30dc54","line":197,"in_reply_to":"3fa7e38b_4d97583a","updated":"2019-11-26 00:03:29.000000000","message":"Ack, will.","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"37ba3586ddeeb7dcb8642fb5decd416da6433a17","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        with self.virtapi.wait_for_instance_event(\u0027instance\u0027,"},{"line_number":196,"context_line":"                                                  [(\u0027foo\u0027, \u0027bar\u0027)]):"},{"line_number":197,"context_line":"            self.virtapi.exit_wait_early([(\u0027foo\u0027, \u0027baz\u0027)])"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def test_update_compute_provider_status(self):"},{"line_number":200,"context_line":"        \"\"\"Tests scenarios for adding/removing the COMPUTE_STATUS_DISABLED"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_4d97583a","line":197,"in_reply_to":"3fa7e38b_9297abeb","updated":"2019-11-25 23:48:58.000000000","message":"Oh, sorry, I didn\u0027t notice \u0027baz\u0027 (as opposed to \u0027bar\u0027). Might make that more visually obvious (but it\u0027s really my fault).","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"977eff52fcf87241bc7e1d4b2cc56ee2d4b1978a","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        with self.virtapi.wait_for_instance_event(\u0027instance\u0027,"},{"line_number":196,"context_line":"                                                  [(\u0027foo\u0027, \u0027bar\u0027)]):"},{"line_number":197,"context_line":"            self.virtapi.exit_wait_early([(\u0027foo\u0027, \u0027baz\u0027)])"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def test_update_compute_provider_status(self):"},{"line_number":200,"context_line":"        \"\"\"Tests scenarios for adding/removing the COMPUTE_STATUS_DISABLED"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_9297abeb","line":197,"in_reply_to":"3fa7e38b_d240c334","updated":"2019-11-25 22:43:20.000000000","message":"Well, this does to be clear, it\u0027s just that I don\u0027t have the baz one in there so it never does anything for that one but does complete the bar one we waited for. But yeah, I will put more assertions here in line with the first test in this series that checks that we called everything in the fake.","commit_id":"1172cc39b4627d2dcf6789b599733b5e91959c01"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"06954e1fbb2216bf83e5863dcfe8b094f1208e38","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                                                  [(\u0027foo\u0027, \u0027bar\u0027),"},{"line_number":196,"context_line":"                                                   (\u0027foo\u0027, \u0027baz\u0027)]):"},{"line_number":197,"context_line":"            self.virtapi.exit_wait_early([(\u0027foo\u0027, \u0027baz\u0027)])"},{"line_number":198,"context_line":"            self.assertEqual(2, len(self.compute._events))"},{"line_number":199,"context_line":"            for event in self.compute._events:"},{"line_number":200,"context_line":"                if event.tag \u003d\u003d \u0027bar\u0027:"},{"line_number":201,"context_line":"                    event.wait.assert_called_once_with()"},{"line_number":202,"context_line":"                else:"},{"line_number":203,"context_line":"                    event.wait.assert_not_called()"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def test_update_compute_provider_status(self):"},{"line_number":206,"context_line":"        \"\"\"Tests scenarios for adding/removing the COMPUTE_STATUS_DISABLED"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_c14785a3","line":203,"range":{"start_line":198,"start_character":0,"end_line":203,"end_character":50},"updated":"2019-11-26 18:35:03.000000000","message":"So my point: none of this code is reached. And you should put a self.fail() here to prove it.\n\nThen dedent these lines so they actually run.","commit_id":"0a9eda254c41fefaab48ac97f5f7be860b080be2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f1c86e0a3c52f1fc282201756250995cf238af93","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                                                  [(\u0027foo\u0027, \u0027bar\u0027),"},{"line_number":196,"context_line":"                                                   (\u0027foo\u0027, \u0027baz\u0027)]):"},{"line_number":197,"context_line":"            self.virtapi.exit_wait_early([(\u0027foo\u0027, \u0027baz\u0027)])"},{"line_number":198,"context_line":"            self.assertEqual(2, len(self.compute._events))"},{"line_number":199,"context_line":"            for event in self.compute._events:"},{"line_number":200,"context_line":"                if event.tag \u003d\u003d \u0027bar\u0027:"},{"line_number":201,"context_line":"                    event.wait.assert_called_once_with()"},{"line_number":202,"context_line":"                else:"},{"line_number":203,"context_line":"                    event.wait.assert_not_called()"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def test_update_compute_provider_status(self):"},{"line_number":206,"context_line":"        \"\"\"Tests scenarios for adding/removing the COMPUTE_STATUS_DISABLED"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_01aa7d19","line":203,"range":{"start_line":198,"start_character":0,"end_line":203,"end_character":50},"in_reply_to":"3fa7e38b_c14785a3","updated":"2019-11-26 18:41:02.000000000","message":"Yup.","commit_id":"0a9eda254c41fefaab48ac97f5f7be860b080be2"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"367c3fc85e4869b3904e567ab0023d4fdb11f165","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                                                  [(\u0027foo\u0027, \u0027bar\u0027),"},{"line_number":196,"context_line":"                                                   (\u0027foo\u0027, \u0027baz\u0027)]):"},{"line_number":197,"context_line":"            self.virtapi.exit_wait_early([(\u0027foo\u0027, \u0027baz\u0027)])"},{"line_number":198,"context_line":"            self.fail(\u0027never gonna happen\u0027)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"        self.assertEqual(2, len(self.compute._events))"},{"line_number":201,"context_line":"        for event in self.compute._events:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_6741ef63","line":198,"range":{"start_line":198,"start_character":35,"end_line":198,"end_character":41},"updated":"2019-11-27 19:29:23.000000000","message":"Aww, I was hoping for \"never gonna give you up\"","commit_id":"9fae8e2eadc770bc59a71ef728df7b980458c257"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bd5ccdda8ef7644cc0d44f84eec3d0aeda421833","unresolved":false,"context_lines":[{"line_number":196,"context_line":"    def test_wait_for_instance_event_exit_early(self):"},{"line_number":197,"context_line":"        # Wait for two events, exit early skipping one."},{"line_number":198,"context_line":"        # Make sure we waited for one and did not wait for the other"},{"line_number":199,"context_line":"        with self.virtapi.wait_for_instance_event(\u0027instance\u0027,"},{"line_number":200,"context_line":"                                                  [(\u0027foo\u0027, \u0027bar\u0027),"},{"line_number":201,"context_line":"                                                   (\u0027foo\u0027, \u0027baz\u0027)]):"},{"line_number":202,"context_line":"            self.virtapi.exit_wait_early([(\u0027foo\u0027, \u0027baz\u0027)])"},{"line_number":203,"context_line":"            self.fail(\u0027never gonna happen\u0027)"},{"line_number":204,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_aaef6eaf","line":201,"range":{"start_line":199,"start_character":9,"end_line":201,"end_character":68},"updated":"2019-12-03 16:44:05.000000000","message":"so here we execute the context manager until we hit the yeild just after starting they try block. https://review.opendev.org/#/c/695985/4/nova/compute/manager.py@502","commit_id":"56d3cd7aa7f4d3be01fd2a5c10903fb548c49458"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bd5ccdda8ef7644cc0d44f84eec3d0aeda421833","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        with self.virtapi.wait_for_instance_event(\u0027instance\u0027,"},{"line_number":200,"context_line":"                                                  [(\u0027foo\u0027, \u0027bar\u0027),"},{"line_number":201,"context_line":"                                                   (\u0027foo\u0027, \u0027baz\u0027)]):"},{"line_number":202,"context_line":"            self.virtapi.exit_wait_early([(\u0027foo\u0027, \u0027baz\u0027)])"},{"line_number":203,"context_line":"            self.fail(\u0027never gonna happen\u0027)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        self.assertEqual(2, len(self.compute._events))"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_2a7ffedf","line":202,"range":{"start_line":202,"start_character":11,"end_line":202,"end_character":58},"updated":"2019-12-03 16:44:05.000000000","message":"then this raise the events as an excpetion.\nhttps://review.opendev.org/#/c/695985/4/nova/compute/manager.py@435\n\nwhich will cause the context manager resume execution\n\nthe event will be pulled out of the payload fo the exception and then we will skip waiting for those events. while still waiting for the rest.","commit_id":"56d3cd7aa7f4d3be01fd2a5c10903fb548c49458"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bd5ccdda8ef7644cc0d44f84eec3d0aeda421833","unresolved":false,"context_lines":[{"line_number":200,"context_line":"                                                  [(\u0027foo\u0027, \u0027bar\u0027),"},{"line_number":201,"context_line":"                                                   (\u0027foo\u0027, \u0027baz\u0027)]):"},{"line_number":202,"context_line":"            self.virtapi.exit_wait_early([(\u0027foo\u0027, \u0027baz\u0027)])"},{"line_number":203,"context_line":"            self.fail(\u0027never gonna happen\u0027)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        self.assertEqual(2, len(self.compute._events))"},{"line_number":206,"context_line":"        for event in self.compute._events:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_8a61523b","line":203,"range":{"start_line":203,"start_character":11,"end_line":203,"end_character":43},"updated":"2019-12-03 16:44:05.000000000","message":"which is why we dont execute this because the exception cause us to exext the context manager before we get there and go to line 205 when it completes skip 203","commit_id":"56d3cd7aa7f4d3be01fd2a5c10903fb548c49458"}],"nova/virt/virtapi.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bd5ccdda8ef7644cc0d44f84eec3d0aeda421833","unresolved":false,"context_lines":[{"line_number":21,"context_line":"                                error_callback\u003dNone):"},{"line_number":22,"context_line":"        raise NotImplementedError()"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    def exit_wait_early(self, events):"},{"line_number":25,"context_line":"        raise NotImplementedError()"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    def update_compute_provider_status(self, context, rp_uuid, enabled):"},{"line_number":28,"context_line":"        \"\"\"Used to add/remove the COMPUTE_STATUS_DISABLED trait on the provider"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_ea1d6610","line":25,"range":{"start_line":24,"start_character":38,"end_line":25,"end_character":8},"updated":"2019-12-03 16:44:05.000000000","message":"thanks for adding this.\ni know we likely will remove the interface class in the future but its good to keep it in sync while its here.","commit_id":"56d3cd7aa7f4d3be01fd2a5c10903fb548c49458"}]}
