)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"77a1daa70783ccd707efca5b0b66664b30737d56","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"The added functional test does not check for a change in behavior. It"},{"line_number":23,"context_line":"ensures that the behavior of delete does not change as the delete"},{"line_number":24,"context_line":"process is updated. Note that the only delete path that bails early is"},{"line_number":25,"context_line":"not yet reachable. The test keeps things honest as that changes."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: I27d89143e8804aebdd414791f03b622abdaafe90"},{"line_number":28,"context_line":"Partially-implements: bp add-buildrequest-obj"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"dada55a8_20f24a06","line":25,"range":{"start_line":24,"start_character":20,"end_line":25,"end_character":64},"updated":"2016-07-22 00:35:44.000000000","message":"Is this why we have no unit tests for the compute API changes here? I don\u0027t really follow.","commit_id":"abbb2ecdb1becaad02006dc2f1388e6c5fd7dc05"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"ea4759e0477b74096ed818e473d1f13490c7933b","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"The added functional test does not check for a change in behavior. It"},{"line_number":23,"context_line":"ensures that the behavior of delete does not change as the delete"},{"line_number":24,"context_line":"process is updated. Note that the only delete path that bails early is"},{"line_number":25,"context_line":"not yet reachable. The test keeps things honest as that changes."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: I27d89143e8804aebdd414791f03b622abdaafe90"},{"line_number":28,"context_line":"Partially-implements: bp add-buildrequest-obj"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"dada55a8_75e84117","line":25,"range":{"start_line":24,"start_character":20,"end_line":25,"end_character":64},"in_reply_to":"dada55a8_20f24a06","updated":"2016-07-23 14:48:34.000000000","message":"There should be tests for the compute api here. I had written some, though I do not know where they have gone. I will fix that as well as addressing the missing delete case you pointed out.","commit_id":"abbb2ecdb1becaad02006dc2f1388e6c5fd7dc05"}],"nova/compute/api.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5da42f442fbb718f381179be77b735c98c478191","unresolved":false,"context_lines":[{"line_number":1536,"context_line":"        # should be the common case."},{"line_number":1537,"context_line":"        # Not having a host does not guarantee the BuildRequest still exists."},{"line_number":1538,"context_line":"        if not instance.host:"},{"line_number":1539,"context_line":"            # If there is a BuildRequest then the instance may not have been"},{"line_number":1540,"context_line":"            # written to a cell db yet. Delete the BuildRequest here, which"},{"line_number":1541,"context_line":"            # will indicate that the Instance build should not proceed."},{"line_number":1542,"context_line":"            # TODO(alaski): handle quotas."}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_b894198b","line":1539,"updated":"2016-06-13 00:48:48.000000000","message":"Agree that a split out private method would be useful in reducing this method\u0027s overall functional complexity.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cf721e31b60e81be89de805322b6b758ec82a53e","unresolved":false,"context_lines":[{"line_number":1536,"context_line":"        # should be the common case."},{"line_number":1537,"context_line":"        # Not having a host does not guarantee the BuildRequest still exists."},{"line_number":1538,"context_line":"        if not instance.host:"},{"line_number":1539,"context_line":"            # If there is a BuildRequest then the instance may not have been"},{"line_number":1540,"context_line":"            # written to a cell db yet. Delete the BuildRequest here, which"},{"line_number":1541,"context_line":"            # will indicate that the Instance build should not proceed."},{"line_number":1542,"context_line":"            # TODO(alaski): handle quotas."}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_6d0855da","line":1539,"updated":"2016-06-10 19:15:10.000000000","message":"Can we pull the rest of this new code into a private method, maybe _delete_instance_not_placed or _delete_no_host? This method has always bugged me how big and complex it is, it\u0027d be good to not add more to it.\n\nThe only downside to pull this into it\u0027s own method is then we\u0027d need to return something to tell this method if it should continue or not which is kind of gross.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"daa6d784f64736a74eee8f4bb01efd5b67cc5231","unresolved":false,"context_lines":[{"line_number":1536,"context_line":"        # should be the common case."},{"line_number":1537,"context_line":"        # Not having a host does not guarantee the BuildRequest still exists."},{"line_number":1538,"context_line":"        if not instance.host:"},{"line_number":1539,"context_line":"            # If there is a BuildRequest then the instance may not have been"},{"line_number":1540,"context_line":"            # written to a cell db yet. Delete the BuildRequest here, which"},{"line_number":1541,"context_line":"            # will indicate that the Instance build should not proceed."},{"line_number":1542,"context_line":"            # TODO(alaski): handle quotas."}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_1373b281","line":1539,"in_reply_to":"7aa08908_6d0855da","updated":"2016-06-10 20:01:27.000000000","message":"Despite the downside I do think this should be put into a separate method for sanity.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5da42f442fbb718f381179be77b735c98c478191","unresolved":false,"context_lines":[{"line_number":1562,"context_line":"                    pass"},{"line_number":1563,"context_line":"                # TODO(alaski): Uncomment this when instance creation happens"},{"line_number":1564,"context_line":"                # in conductor and deleting BuildRequest will halt it."},{"line_number":1565,"context_line":"                # return"},{"line_number":1566,"context_line":"                try:"},{"line_number":1567,"context_line":"                    instance \u003d self.get(context, instance.uuid,"},{"line_number":1568,"context_line":"                                        want_objects\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_188e0d79","line":1565,"updated":"2016-06-13 00:48:48.000000000","message":"Extraneous/erroneous \"return\" above? :)","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d654b38b72c3ad0fe8ddc490e80ce14f09160af5","unresolved":false,"context_lines":[{"line_number":1562,"context_line":"                    pass"},{"line_number":1563,"context_line":"                # TODO(alaski): Uncomment this when instance creation happens"},{"line_number":1564,"context_line":"                # in conductor and deleting BuildRequest will halt it."},{"line_number":1565,"context_line":"                # return"},{"line_number":1566,"context_line":"                try:"},{"line_number":1567,"context_line":"                    instance \u003d self.get(context, instance.uuid,"},{"line_number":1568,"context_line":"                                        want_objects\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a9d85d2_d082c751","line":1565,"in_reply_to":"5a9d85d2_9214e3f5","updated":"2016-06-21 13:01:42.000000000","message":"Yeah, the TODO was to uncomment this at a later appropriate time.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4ed61e46490745aff2b2800438be066bc8ae443c","unresolved":false,"context_lines":[{"line_number":1562,"context_line":"                    pass"},{"line_number":1563,"context_line":"                # TODO(alaski): Uncomment this when instance creation happens"},{"line_number":1564,"context_line":"                # in conductor and deleting BuildRequest will halt it."},{"line_number":1565,"context_line":"                # return"},{"line_number":1566,"context_line":"                try:"},{"line_number":1567,"context_line":"                    instance \u003d self.get(context, instance.uuid,"},{"line_number":1568,"context_line":"                                        want_objects\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a9d85d2_9214e3f5","line":1565,"in_reply_to":"7aa08908_188e0d79","updated":"2016-06-21 00:11:52.000000000","message":"I assume it\u0027s part of the TODO?","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cf721e31b60e81be89de805322b6b758ec82a53e","unresolved":false,"context_lines":[{"line_number":1564,"context_line":"                # in conductor and deleting BuildRequest will halt it."},{"line_number":1565,"context_line":"                # return"},{"line_number":1566,"context_line":"                try:"},{"line_number":1567,"context_line":"                    instance \u003d self.get(context, instance.uuid,"},{"line_number":1568,"context_line":"                                        want_objects\u003dTrue)"},{"line_number":1569,"context_line":"                except exception.InstanceNotFound:"},{"line_number":1570,"context_line":"                    # If no instance was found then it is truly deleted. If an"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_6dbfb560","line":1567,"updated":"2016-06-10 19:15:10.000000000","message":"Is this just an existence check or do you actually want to refresh the instance variable that was passed in here? The reason I ask is get() adds the standard expected_attrs so it\u0027ll do the big join in the DB, which if you\u0027re just doing an existence check we could avoid with just calling objects.Instance.get_by_uuid() right here.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"daa6d784f64736a74eee8f4bb01efd5b67cc5231","unresolved":false,"context_lines":[{"line_number":1564,"context_line":"                # in conductor and deleting BuildRequest will halt it."},{"line_number":1565,"context_line":"                # return"},{"line_number":1566,"context_line":"                try:"},{"line_number":1567,"context_line":"                    instance \u003d self.get(context, instance.uuid,"},{"line_number":1568,"context_line":"                                        want_objects\u003dTrue)"},{"line_number":1569,"context_line":"                except exception.InstanceNotFound:"},{"line_number":1570,"context_line":"                    # If no instance was found then it is truly deleted. If an"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_b3ebbe77","line":1567,"in_reply_to":"7aa08908_6dbfb560","updated":"2016-06-10 20:01:27.000000000","message":"It is an existence check, but it\u0027s also pulling the instance for the rest of the delete path below. And some conditions could lead to needing the joined data, like line 1591 below.\n\nWe could take a chance and lazy-load them if necessary. I guess it depends on if the most used path needs any expected_attrs.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cf721e31b60e81be89de805322b6b758ec82a53e","unresolved":false,"context_lines":[{"line_number":1565,"context_line":"                # return"},{"line_number":1566,"context_line":"                try:"},{"line_number":1567,"context_line":"                    instance \u003d self.get(context, instance.uuid,"},{"line_number":1568,"context_line":"                                        want_objects\u003dTrue)"},{"line_number":1569,"context_line":"                except exception.InstanceNotFound:"},{"line_number":1570,"context_line":"                    # If no instance was found then it is truly deleted. If an"},{"line_number":1571,"context_line":"                    # instance is found then instance creation has not moved to"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_0d40b1a4","line":1568,"range":{"start_line":1568,"start_character":40,"end_line":1568,"end_character":57},"updated":"2016-06-10 19:15:10.000000000","message":"Why I can\u0027t believe this isn\u0027t the default yet.\n\nNote to self, clean up anything still not using want_objects\u003dTrue.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"daa6d784f64736a74eee8f4bb01efd5b67cc5231","unresolved":false,"context_lines":[{"line_number":1565,"context_line":"                # return"},{"line_number":1566,"context_line":"                try:"},{"line_number":1567,"context_line":"                    instance \u003d self.get(context, instance.uuid,"},{"line_number":1568,"context_line":"                                        want_objects\u003dTrue)"},{"line_number":1569,"context_line":"                except exception.InstanceNotFound:"},{"line_number":1570,"context_line":"                    # If no instance was found then it is truly deleted. If an"},{"line_number":1571,"context_line":"                    # instance is found then instance creation has not moved to"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_f37266e2","line":1568,"range":{"start_line":1568,"start_character":40,"end_line":1568,"end_character":57},"in_reply_to":"7aa08908_0d40b1a4","updated":"2016-06-10 20:01:27.000000000","message":"I was surprised too. I would +2 a cleanup so hard.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5da42f442fbb718f381179be77b735c98c478191","unresolved":false,"context_lines":[{"line_number":1570,"context_line":"                    # If no instance was found then it is truly deleted. If an"},{"line_number":1571,"context_line":"                    # instance is found then instance creation has not moved to"},{"line_number":1572,"context_line":"                    # the conductor yet."},{"line_number":1573,"context_line":"                    # Quota handling needs to happen before bailing early here."},{"line_number":1574,"context_line":"                    project_id, user_id \u003d quotas_obj.ids_from_instance("},{"line_number":1575,"context_line":"                        context, instance)"},{"line_number":1576,"context_line":"                    quotas \u003d self._create_reservations(context,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_f88a2163","line":1573,"updated":"2016-06-13 00:48:48.000000000","message":"I don\u0027t see any bailing early happening... is the above comment correct? Also, I\u0027m a little confused about two things. First, why are we committing quotas here if the instance exists? This is the destroy() method, no? Secondly, you are handling quotas here but the TODO above on line 1542 indicates that you still need to handle quotas...","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d61850327aa508e07e72b80d380859509963a614","unresolved":false,"context_lines":[{"line_number":1570,"context_line":"                    # If no instance was found then it is truly deleted. If an"},{"line_number":1571,"context_line":"                    # instance is found then instance creation has not moved to"},{"line_number":1572,"context_line":"                    # the conductor yet."},{"line_number":1573,"context_line":"                    # Quota handling needs to happen before bailing early here."},{"line_number":1574,"context_line":"                    project_id, user_id \u003d quotas_obj.ids_from_instance("},{"line_number":1575,"context_line":"                        context, instance)"},{"line_number":1576,"context_line":"                    quotas \u003d self._create_reservations(context,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_581b5c73","line":1573,"in_reply_to":"7aa08908_f88a2163","updated":"2016-06-13 14:22:37.000000000","message":"The return on line 1581 below is the \"bail early\" bit. This decrements quotas. _create_reservations is extremely poorly named and is only used in the delete case and actually decrements quotas. Surprised me too.\n\nYep, the TODO needs to be removed.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5da42f442fbb718f381179be77b735c98c478191","unresolved":false,"context_lines":[{"line_number":1578,"context_line":"                                                       instance.task_state,"},{"line_number":1579,"context_line":"                                                       project_id, user_id)"},{"line_number":1580,"context_line":"                    quotas.commit()"},{"line_number":1581,"context_line":"                    return"},{"line_number":1582,"context_line":""},{"line_number":1583,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":1584,"context_line":"                context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_58ea5510","line":1581,"updated":"2016-06-13 00:48:48.000000000","message":"femtonit: return not needed..","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d61850327aa508e07e72b80d380859509963a614","unresolved":false,"context_lines":[{"line_number":1578,"context_line":"                                                       instance.task_state,"},{"line_number":1579,"context_line":"                                                       project_id, user_id)"},{"line_number":1580,"context_line":"                    quotas.commit()"},{"line_number":1581,"context_line":"                    return"},{"line_number":1582,"context_line":""},{"line_number":1583,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":1584,"context_line":"                context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_58323cf3","line":1581,"in_reply_to":"7aa08908_58ea5510","updated":"2016-06-13 14:22:37.000000000","message":"If we\u0027ve hit this exception case then there is no instance in the db and we\u0027ll error in the below code. None of it is necessary to run in this case so we can just bail early.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6ac51a327bda3b429a6ffefac326e3dac5fd693d","unresolved":false,"context_lines":[{"line_number":1595,"context_line":"                    # If no instance was found then it is truly deleted. If an"},{"line_number":1596,"context_line":"                    # instance is found then instance creation has not moved to"},{"line_number":1597,"context_line":"                    # the conductor yet."},{"line_number":1598,"context_line":"                    project_id, user_id \u003d quotas_obj.ids_from_instance("},{"line_number":1599,"context_line":"                        context, instance)"},{"line_number":1600,"context_line":"                    quotas \u003d self._create_reservations(context,"},{"line_number":1601,"context_line":"                                                       instance,"},{"line_number":1602,"context_line":"                                                       instance.task_state,"},{"line_number":1603,"context_line":"                                                       project_id, user_id)"},{"line_number":1604,"context_line":"                    quotas.commit()"},{"line_number":1605,"context_line":"                    return True"},{"line_number":1606,"context_line":"        return False"},{"line_number":1607,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5a9d85d2_f27727d2","line":1604,"range":{"start_line":1598,"start_character":20,"end_line":1604,"end_character":35},"updated":"2016-06-21 00:51:44.000000000","message":"Might want a comment about why we have to do this.","commit_id":"1133d2f9586f0c1706820943db7ba4fecf48b5d9"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"a7615f3099ed4cf8e1e5e767aefd67e4c8493a61","unresolved":false,"context_lines":[{"line_number":1595,"context_line":"                    # If no instance was found then it is truly deleted. If an"},{"line_number":1596,"context_line":"                    # instance is found then instance creation has not moved to"},{"line_number":1597,"context_line":"                    # the conductor yet."},{"line_number":1598,"context_line":"                    project_id, user_id \u003d quotas_obj.ids_from_instance("},{"line_number":1599,"context_line":"                        context, instance)"},{"line_number":1600,"context_line":"                    quotas \u003d self._create_reservations(context,"},{"line_number":1601,"context_line":"                                                       instance,"},{"line_number":1602,"context_line":"                                                       instance.task_state,"},{"line_number":1603,"context_line":"                                                       project_id, user_id)"},{"line_number":1604,"context_line":"                    quotas.commit()"},{"line_number":1605,"context_line":"                    return True"},{"line_number":1606,"context_line":"        return False"},{"line_number":1607,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5a9d85d2_10f44ff0","line":1604,"range":{"start_line":1598,"start_character":20,"end_line":1604,"end_character":35},"in_reply_to":"5a9d85d2_f27727d2","updated":"2016-06-21 13:02:58.000000000","message":"Can do.","commit_id":"1133d2f9586f0c1706820943db7ba4fecf48b5d9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6ac51a327bda3b429a6ffefac326e3dac5fd693d","unresolved":false,"context_lines":[{"line_number":1616,"context_line":"            return"},{"line_number":1617,"context_line":"        # If the instance came from a BuildRequest it will be orphaned. Look"},{"line_number":1618,"context_line":"        # it up."},{"line_number":1619,"context_line":"        try:"},{"line_number":1620,"context_line":"            inst_map \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":1621,"context_line":"                context, instance.uuid)"},{"line_number":1622,"context_line":"        except exception.InstanceMappingNotFound:"},{"line_number":1623,"context_line":"            # Not on cellsv2 yet, try looking up the instance."},{"line_number":1624,"context_line":"            try:"},{"line_number":1625,"context_line":"                instance \u003d objects.Instance.get_by_uuid(context, instance.uuid)"},{"line_number":1626,"context_line":"            except exception.InstanceNotFound:"},{"line_number":1627,"context_line":"                # Instance is deleted"},{"line_number":1628,"context_line":"                return"},{"line_number":1629,"context_line":"        else:"},{"line_number":1630,"context_line":"            with nova_context.target_cell(context, inst_map.cell_mapping):"},{"line_number":1631,"context_line":"                try:"},{"line_number":1632,"context_line":"                    instance \u003d objects.Instance.get_by_uuid(context,"},{"line_number":1633,"context_line":"                                                            instance.uuid)"},{"line_number":1634,"context_line":"                except exception.InstanceNotFound:"},{"line_number":1635,"context_line":"                    # Instance is deleted"},{"line_number":1636,"context_line":"                    return"},{"line_number":1637,"context_line":""},{"line_number":1638,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":1639,"context_line":"                context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a9d85d2_8d468221","line":1636,"range":{"start_line":1619,"start_character":8,"end_line":1636,"end_character":26},"updated":"2016-06-21 00:51:44.000000000","message":"Seems that we\u0027re missing unit tests for these goodies.","commit_id":"1133d2f9586f0c1706820943db7ba4fecf48b5d9"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"a7615f3099ed4cf8e1e5e767aefd67e4c8493a61","unresolved":false,"context_lines":[{"line_number":1616,"context_line":"            return"},{"line_number":1617,"context_line":"        # If the instance came from a BuildRequest it will be orphaned. Look"},{"line_number":1618,"context_line":"        # it up."},{"line_number":1619,"context_line":"        try:"},{"line_number":1620,"context_line":"            inst_map \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":1621,"context_line":"                context, instance.uuid)"},{"line_number":1622,"context_line":"        except exception.InstanceMappingNotFound:"},{"line_number":1623,"context_line":"            # Not on cellsv2 yet, try looking up the instance."},{"line_number":1624,"context_line":"            try:"},{"line_number":1625,"context_line":"                instance \u003d objects.Instance.get_by_uuid(context, instance.uuid)"},{"line_number":1626,"context_line":"            except exception.InstanceNotFound:"},{"line_number":1627,"context_line":"                # Instance is deleted"},{"line_number":1628,"context_line":"                return"},{"line_number":1629,"context_line":"        else:"},{"line_number":1630,"context_line":"            with nova_context.target_cell(context, inst_map.cell_mapping):"},{"line_number":1631,"context_line":"                try:"},{"line_number":1632,"context_line":"                    instance \u003d objects.Instance.get_by_uuid(context,"},{"line_number":1633,"context_line":"                                                            instance.uuid)"},{"line_number":1634,"context_line":"                except exception.InstanceNotFound:"},{"line_number":1635,"context_line":"                    # Instance is deleted"},{"line_number":1636,"context_line":"                    return"},{"line_number":1637,"context_line":""},{"line_number":1638,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":1639,"context_line":"                context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a9d85d2_10428f10","line":1636,"range":{"start_line":1619,"start_character":8,"end_line":1636,"end_character":26},"in_reply_to":"5a9d85d2_8d468221","updated":"2016-06-21 13:02:58.000000000","message":"Yeah, I\u0027ll add some for this.","commit_id":"1133d2f9586f0c1706820943db7ba4fecf48b5d9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e639d36deadcb73ded4193e13e8cb7e8d92abf07","unresolved":false,"context_lines":[{"line_number":1538,"context_line":""},{"line_number":1539,"context_line":"        Deletion before an instance reaches a cell needs to be handled"},{"line_number":1540,"context_line":"        differently. What we\u0027re attempting to do is delete the BuildRequest"},{"line_number":1541,"context_line":"        before the conductor does. If we succeed here then the boot request"},{"line_number":1542,"context_line":"        stops before reaching a cell. If not then the instance will need to"},{"line_number":1543,"context_line":"        be looked up in a cell db and the normal delete path taken."},{"line_number":1544,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":17,"id":"dada55a8_02099c31","line":1541,"updated":"2016-07-19 16:28:33.000000000","message":"Before a *semiconductor* does? :) The \"conductor\" is actually in a cell, right? So, if a boot request has reached a conductor, it\u0027s already in a cell, no?","commit_id":"e909bc7252648859e09319b0b0bc57a11e942418"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1c928d5b3a2ec6f4ab2ca9800fb93255e0fbc272","unresolved":false,"context_lines":[{"line_number":1538,"context_line":""},{"line_number":1539,"context_line":"        Deletion before an instance reaches a cell needs to be handled"},{"line_number":1540,"context_line":"        differently. What we\u0027re attempting to do is delete the BuildRequest"},{"line_number":1541,"context_line":"        before the conductor does. If we succeed here then the boot request"},{"line_number":1542,"context_line":"        stops before reaching a cell. If not then the instance will need to"},{"line_number":1543,"context_line":"        be looked up in a cell db and the normal delete path taken."},{"line_number":1544,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":17,"id":"dada55a8_211f3d09","line":1541,"in_reply_to":"dada55a8_02099c31","updated":"2016-07-20 17:34:29.000000000","message":"Yes. Now that we are calling it semiconductor we can distinguish between the two.","commit_id":"e909bc7252648859e09319b0b0bc57a11e942418"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e639d36deadcb73ded4193e13e8cb7e8d92abf07","unresolved":false,"context_lines":[{"line_number":1557,"context_line":""},{"line_number":1558,"context_line":"        deleted \u003d self._attempt_delete_of_buildrequest(context, instance)"},{"line_number":1559,"context_line":"        if deleted:"},{"line_number":1560,"context_line":"            # The boot process has been halted before reaching compute so"},{"line_number":1561,"context_line":"            # the instance does not need cleaning up."},{"line_number":1562,"context_line":"            # Quotas must be decremented."},{"line_number":1563,"context_line":"            project_id, user_id \u003d quotas_obj.ids_from_instance("}],"source_content_type":"text/x-python","patch_set":17,"id":"dada55a8_6208582d","line":1560,"updated":"2016-07-19 16:28:33.000000000","message":"Before reaching the compute or before reaching the cell conductor?","commit_id":"e909bc7252648859e09319b0b0bc57a11e942418"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1c928d5b3a2ec6f4ab2ca9800fb93255e0fbc272","unresolved":false,"context_lines":[{"line_number":1557,"context_line":""},{"line_number":1558,"context_line":"        deleted \u003d self._attempt_delete_of_buildrequest(context, instance)"},{"line_number":1559,"context_line":"        if deleted:"},{"line_number":1560,"context_line":"            # The boot process has been halted before reaching compute so"},{"line_number":1561,"context_line":"            # the instance does not need cleaning up."},{"line_number":1562,"context_line":"            # Quotas must be decremented."},{"line_number":1563,"context_line":"            project_id, user_id \u003d quotas_obj.ids_from_instance("}],"source_content_type":"text/x-python","patch_set":17,"id":"dada55a8_c1a2a1ad","line":1560,"in_reply_to":"dada55a8_6208582d","updated":"2016-07-20 17:34:29.000000000","message":"Compute. The build request goes from nova-api -\u003e nova-semiconductor (call scheduler) -\u003e nova-compute. It does not go through the cell conductor for a boot. A failure on a compute will send the request to the cell conductor for a reschedule.","commit_id":"e909bc7252648859e09319b0b0bc57a11e942418"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e639d36deadcb73ded4193e13e8cb7e8d92abf07","unresolved":false,"context_lines":[{"line_number":1622,"context_line":"            return"},{"line_number":1623,"context_line":""},{"line_number":1624,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":1625,"context_line":"                context, instance.uuid)"},{"line_number":1626,"context_line":""},{"line_number":1627,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":1628,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"dada55a8_4203940f","line":1625,"updated":"2016-07-19 16:28:33.000000000","message":"Not a change that this patch needs to do, but eventually, it would be great to have transactionally-related cleanup DB actions like this one be done within instance.destroy() instead of in the compute API...","commit_id":"e909bc7252648859e09319b0b0bc57a11e942418"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"77a1daa70783ccd707efca5b0b66664b30737d56","unresolved":false,"context_lines":[{"line_number":1538,"context_line":""},{"line_number":1539,"context_line":"        Deletion before an instance reaches a cell needs to be handled"},{"line_number":1540,"context_line":"        differently. What we\u0027re attempting to do is delete the BuildRequest"},{"line_number":1541,"context_line":"        before the api level conductor does, hereafter known as semiconductor."},{"line_number":1542,"context_line":"        If we succeed here then the boot request stops before reaching a cell."},{"line_number":1543,"context_line":"        If not then the instance will need to be looked up in a cell db and the"},{"line_number":1544,"context_line":"        normal delete path taken."}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_45003034","line":1541,"range":{"start_line":1541,"start_character":64,"end_line":1541,"end_character":77},"updated":"2016-07-22 00:35:44.000000000","message":"oh boy","commit_id":"abbb2ecdb1becaad02006dc2f1388e6c5fd7dc05"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"77a1daa70783ccd707efca5b0b66664b30737d56","unresolved":false,"context_lines":[{"line_number":1558,"context_line":""},{"line_number":1559,"context_line":"        deleted \u003d self._attempt_delete_of_buildrequest(context, instance)"},{"line_number":1560,"context_line":"        if deleted:"},{"line_number":1561,"context_line":"            # The boot process has been halted before reaching compute so"},{"line_number":1562,"context_line":"            # the instance does not need cleaning up."},{"line_number":1563,"context_line":"            # Quotas must be decremented."},{"line_number":1564,"context_line":"            project_id, user_id \u003d quotas_obj.ids_from_instance("},{"line_number":1565,"context_line":"                context, instance)"}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_60444249","line":1562,"range":{"start_line":1561,"start_character":14,"end_line":1562,"end_character":53},"updated":"2016-07-22 00:35:44.000000000","message":"I\u0027m a bit lost here. There is a window where the BuildRequest and the Instance can both exist in _provision_instances in the compute API. Say we\u0027re looping over a request to create 10 instances and we\u0027ve created 5 in the DB but we\u0027re still chugging along. Then I put in a request to delete the first one. This code will delete the BuildRequest but not the Instance record, so when I list/show the instance it\u0027s still going to show up. It wouldn\u0027t have been created on the compute yet, so that\u0027s good, but the API results would be weird.\n\nGranted, this is a screwy scenario since I would have had to perform my delete request in parallel before my create request returned with a 202, but it\u0027s possible isn\u0027t it? Or am I missing something? Would it be worth also looking up the instance to delete it even if we delete the BuildRequest?\n\nWhile I\u0027m looking, shouldn\u0027t we also delete the BuildRequests we\u0027ve created in _provision_instances if something fails, like we do for the instances we\u0027ve created, here?\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L1000","commit_id":"abbb2ecdb1becaad02006dc2f1388e6c5fd7dc05"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"ea4759e0477b74096ed818e473d1f13490c7933b","unresolved":false,"context_lines":[{"line_number":1558,"context_line":""},{"line_number":1559,"context_line":"        deleted \u003d self._attempt_delete_of_buildrequest(context, instance)"},{"line_number":1560,"context_line":"        if deleted:"},{"line_number":1561,"context_line":"            # The boot process has been halted before reaching compute so"},{"line_number":1562,"context_line":"            # the instance does not need cleaning up."},{"line_number":1563,"context_line":"            # Quotas must be decremented."},{"line_number":1564,"context_line":"            project_id, user_id \u003d quotas_obj.ids_from_instance("},{"line_number":1565,"context_line":"                context, instance)"}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_95805548","line":1562,"range":{"start_line":1561,"start_character":14,"end_line":1562,"end_character":53},"in_reply_to":"dada55a8_60444249","updated":"2016-07-23 14:48:34.000000000","message":"I think I am missing a case here. But a piece you may be missing is https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L500. The plan is that the successful removal of the BuildRequest here will signal the conductor to not create an instance record in a cell, so the instance is effectively deleted.\n\nHowever there is nothing now that will cause an instance record to be set to deleted in this case. I should add some instance delete code here, or have this return the instance so the rest of the delete code will take affect.","commit_id":"abbb2ecdb1becaad02006dc2f1388e6c5fd7dc05"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c688100b8b1a157a1ad1f8102dbad5552ae14776","unresolved":false,"context_lines":[{"line_number":1585,"context_line":""},{"line_number":1586,"context_line":"        Deletion before an instance reaches a cell needs to be handled"},{"line_number":1587,"context_line":"        differently. What we\u0027re attempting to do is delete the BuildRequest"},{"line_number":1588,"context_line":"        before the api level conductor, hereafter known as semiconductor, does."},{"line_number":1589,"context_line":"        If we succeed here then the boot request stops before reaching a cell."},{"line_number":1590,"context_line":"        If not then the instance will need to be looked up in a cell db and the"},{"line_number":1591,"context_line":"        normal delete path taken."}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_6052ba14","line":1588,"range":{"start_line":1588,"start_character":40,"end_line":1588,"end_character":72},"updated":"2016-08-05 13:49:24.000000000","message":"FTR, I kind of don\u0027t like this unless we\u0027re seriously going to be referring to the API level conductor as semiconductor now. I might just be a curmudgeon though.","commit_id":"1c0334e7182a46e24121ef06a3d0f6f313e11113"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"586a21ca02260c4b9ba215b7b169592a590ddca4","unresolved":false,"context_lines":[{"line_number":1585,"context_line":""},{"line_number":1586,"context_line":"        Deletion before an instance reaches a cell needs to be handled"},{"line_number":1587,"context_line":"        differently. What we\u0027re attempting to do is delete the BuildRequest"},{"line_number":1588,"context_line":"        before the api level conductor, hereafter known as semiconductor, does."},{"line_number":1589,"context_line":"        If we succeed here then the boot request stops before reaching a cell."},{"line_number":1590,"context_line":"        If not then the instance will need to be looked up in a cell db and the"},{"line_number":1591,"context_line":"        normal delete path taken."}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_e05b4a52","line":1588,"range":{"start_line":1588,"start_character":40,"end_line":1588,"end_character":72},"in_reply_to":"bacf61ea_6052ba14","updated":"2016-08-05 13:57:32.000000000","message":"This came out of a discussion between Mr. Pipes and myself and the distinction we made was that the tasks part of the conductor, as opposed to the db proxy, could be the semiconductor service. So the API and cell level could each have a semiconductor.\n\nBut honestly if we\u0027re going to do that it needs more discussion to get everyone on the same page so I\u0027ll remove this for now :)","commit_id":"1c0334e7182a46e24121ef06a3d0f6f313e11113"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c688100b8b1a157a1ad1f8102dbad5552ae14776","unresolved":false,"context_lines":[{"line_number":1627,"context_line":"                if instance is not None:"},{"line_number":1628,"context_line":"                    # If instance is None it has already been deleted."},{"line_number":1629,"context_line":"                    instance.destroy()"},{"line_number":1630,"context_line":"            except exception.InstanceNotFound:"},{"line_number":1631,"context_line":"                pass"},{"line_number":1632,"context_line":""},{"line_number":1633,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_80bc0edd","line":1630,"updated":"2016-08-05 13:49:24.000000000","message":"I don\u0027t think we actually would get here because of L1565.","commit_id":"1c0334e7182a46e24121ef06a3d0f6f313e11113"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"ee17dba76d553646bcf0c812f5f290273afb62fe","unresolved":false,"context_lines":[{"line_number":1627,"context_line":"                if instance is not None:"},{"line_number":1628,"context_line":"                    # If instance is None it has already been deleted."},{"line_number":1629,"context_line":"                    instance.destroy()"},{"line_number":1630,"context_line":"            except exception.InstanceNotFound:"},{"line_number":1631,"context_line":"                pass"},{"line_number":1632,"context_line":""},{"line_number":1633,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_a3595cff","line":1630,"in_reply_to":"bacf61ea_2071e20c","updated":"2016-08-05 14:17:08.000000000","message":"Oh, this is for the destroy() call too. However if the exception is raised that means we hit a race with another process deleting the instance and just letting the exception bubble up should be fine.","commit_id":"1c0334e7182a46e24121ef06a3d0f6f313e11113"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"586a21ca02260c4b9ba215b7b169592a590ddca4","unresolved":false,"context_lines":[{"line_number":1627,"context_line":"                if instance is not None:"},{"line_number":1628,"context_line":"                    # If instance is None it has already been deleted."},{"line_number":1629,"context_line":"                    instance.destroy()"},{"line_number":1630,"context_line":"            except exception.InstanceNotFound:"},{"line_number":1631,"context_line":"                pass"},{"line_number":1632,"context_line":""},{"line_number":1633,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_2071e20c","line":1630,"in_reply_to":"bacf61ea_80bc0edd","updated":"2016-08-05 13:57:32.000000000","message":"Yep, this is from an earlier attempt at things. I\u0027ll remove it.","commit_id":"1c0334e7182a46e24121ef06a3d0f6f313e11113"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c688100b8b1a157a1ad1f8102dbad5552ae14776","unresolved":false,"context_lines":[{"line_number":1642,"context_line":"                context, instance.uuid)"},{"line_number":1643,"context_line":"            build_req.destroy()"},{"line_number":1644,"context_line":"        except exception.BuildRequestNotFound:"},{"line_number":1645,"context_line":"            # This means that conductor has deleted the BuildRequest so the"},{"line_number":1646,"context_line":"            # instance is now in a cell and the delete needs to proceed"},{"line_number":1647,"context_line":"            # normally."},{"line_number":1648,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_00e01e28","line":1645,"range":{"start_line":1645,"start_character":30,"end_line":1645,"end_character":39},"updated":"2016-08-05 13:49:24.000000000","message":"The conductor or the semiconductor? See what I mean? :)","commit_id":"1c0334e7182a46e24121ef06a3d0f6f313e11113"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8d016cbe74a44def5a5b7a6d933adae26c229c8","unresolved":false,"context_lines":[{"line_number":1658,"context_line":"        # sent to a cell/compute which means it was pulled from the cell db."},{"line_number":1659,"context_line":"        # Normal delete should be attempted."},{"line_number":1660,"context_line":"        if not instance.host:"},{"line_number":1661,"context_line":"            # If"},{"line_number":1662,"context_line":"            if self._delete_while_booting(context, instance):"},{"line_number":1663,"context_line":"                return"},{"line_number":1664,"context_line":"            # If instance.host was not set it\u0027s possible that the Instance"}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_d24ffb8f","line":1661,"updated":"2016-08-05 13:41:33.000000000","message":"oops","commit_id":"1c0334e7182a46e24121ef06a3d0f6f313e11113"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"586a21ca02260c4b9ba215b7b169592a590ddca4","unresolved":false,"context_lines":[{"line_number":1658,"context_line":"        # sent to a cell/compute which means it was pulled from the cell db."},{"line_number":1659,"context_line":"        # Normal delete should be attempted."},{"line_number":1660,"context_line":"        if not instance.host:"},{"line_number":1661,"context_line":"            # If"},{"line_number":1662,"context_line":"            if self._delete_while_booting(context, instance):"},{"line_number":1663,"context_line":"                return"},{"line_number":1664,"context_line":"            # If instance.host was not set it\u0027s possible that the Instance"}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_e0518a63","line":1661,"in_reply_to":"bacf61ea_d24ffb8f","updated":"2016-08-05 13:57:32.000000000","message":"Heh, will remove.","commit_id":"1c0334e7182a46e24121ef06a3d0f6f313e11113"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bde2c9dcb16ea4dcc3273bb0cb870cff74d95094","unresolved":false,"context_lines":[{"line_number":1666,"context_line":"            # fully populated. Notably it will be missing an \u0027id\u0027 field which"},{"line_number":1667,"context_line":"            # will prevent instance.destroy from functioning properly. A"},{"line_number":1668,"context_line":"            # lookup is attempted which will either return a full Instance or"},{"line_number":1669,"context_line":"            # will raise InstanceNotFound. If not found then it\u0027s acceptable"},{"line_number":1670,"context_line":"            # to skip the rest of the delete processing."},{"line_number":1671,"context_line":"            instance \u003d self._lookup_instance(context, instance.uuid)"},{"line_number":1672,"context_line":"            if not instance:"}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_20d562e4","line":1669,"range":{"start_line":1669,"start_character":14,"end_line":1669,"end_character":42},"updated":"2016-08-05 13:51:39.000000000","message":"I don\u0027t think this actually gets raised out of _lookup_instance, it\u0027s an internal detail of that method which indicates None will be returned.","commit_id":"1c0334e7182a46e24121ef06a3d0f6f313e11113"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"6542dd48a8dfedd332f953db3e7b44b6a2f4cd00","unresolved":false,"context_lines":[{"line_number":1605,"context_line":"            # instance is necessary, but quotas must still be decremented."},{"line_number":1606,"context_line":"            project_id, user_id \u003d quotas_obj.ids_from_instance("},{"line_number":1607,"context_line":"                context, instance)"},{"line_number":1608,"context_line":"            # This is confusing but actually decrements quota."},{"line_number":1609,"context_line":"            quotas \u003d self._create_reservations(context,"},{"line_number":1610,"context_line":"                                               instance,"},{"line_number":1611,"context_line":"                                               instance.task_state,"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_3bb3c899","line":1608,"updated":"2016-08-05 17:27:59.000000000","message":"*blink*","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e83be08dc7be46b272e4c2b46bbf69839f053e30","unresolved":false,"context_lines":[{"line_number":1610,"context_line":"                                               instance,"},{"line_number":1611,"context_line":"                                               instance.task_state,"},{"line_number":1612,"context_line":"                                               project_id, user_id)"},{"line_number":1613,"context_line":"            quotas.commit()"},{"line_number":1614,"context_line":""},{"line_number":1615,"context_line":"            # NOTE(alaski): Though the conductor halts the build process it"},{"line_number":1616,"context_line":"            # does not currently delete the instance record. This is because in"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_55c3ee37","line":1613,"updated":"2016-08-05 15:45:34.000000000","message":"What if we race to do this? Presumably this fails instead of double-decrements, right? Should we catch this?","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"49ac46d178536a9046c1c4a257d64a83cb44bde6","unresolved":false,"context_lines":[{"line_number":1610,"context_line":"                                               instance,"},{"line_number":1611,"context_line":"                                               instance.task_state,"},{"line_number":1612,"context_line":"                                               project_id, user_id)"},{"line_number":1613,"context_line":"            quotas.commit()"},{"line_number":1614,"context_line":""},{"line_number":1615,"context_line":"            # NOTE(alaski): Though the conductor halts the build process it"},{"line_number":1616,"context_line":"            # does not currently delete the instance record. This is because in"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_1693395c","line":1613,"in_reply_to":"bacf61ea_16117952","updated":"2016-08-05 17:34:26.000000000","message":"We race against a normal delete path. And yeah, all I meant was to wrap this so that we don\u0027t explode.","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"775a992f53bb33c44a5f5118bcf2c8b2a97d74c4","unresolved":false,"context_lines":[{"line_number":1610,"context_line":"                                               instance,"},{"line_number":1611,"context_line":"                                               instance.task_state,"},{"line_number":1612,"context_line":"                                               project_id, user_id)"},{"line_number":1613,"context_line":"            quotas.commit()"},{"line_number":1614,"context_line":""},{"line_number":1615,"context_line":"            # NOTE(alaski): Though the conductor halts the build process it"},{"line_number":1616,"context_line":"            # does not currently delete the instance record. This is because in"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_16117952","line":1613,"in_reply_to":"bacf61ea_55c3ee37","updated":"2016-08-05 17:31:53.000000000","message":"What would we race against, another delete request? This block can only be entered once because the buildrequest can only be deleted once.\n\nI think I see a potential race, though it\u0027s unlikely. To guard against it I should be able to wrap this method call and the instance.destroy below in a try/except and in the case of InstanceNotFound rollback the quotas.commit. That\u0027s how it\u0027s guarded against in _delete().","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e83be08dc7be46b272e4c2b46bbf69839f053e30","unresolved":false,"context_lines":[{"line_number":1625,"context_line":"            instance \u003d self._lookup_instance(context, instance.uuid)"},{"line_number":1626,"context_line":"            if instance is not None:"},{"line_number":1627,"context_line":"                # If instance is None it has already been deleted."},{"line_number":1628,"context_line":"                instance.destroy()"},{"line_number":1629,"context_line":""},{"line_number":1630,"context_line":"            return True"},{"line_number":1631,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_f5a02292","line":1628,"updated":"2016-08-05 15:45:34.000000000","message":"This will explode if you race between the lookup and destroy.","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8e0ed14fd90a9c84dbe1d53781844d397b4546f","unresolved":false,"context_lines":[{"line_number":1625,"context_line":"            instance \u003d self._lookup_instance(context, instance.uuid)"},{"line_number":1626,"context_line":"            if instance is not None:"},{"line_number":1627,"context_line":"                # If instance is None it has already been deleted."},{"line_number":1628,"context_line":"                instance.destroy()"},{"line_number":1629,"context_line":""},{"line_number":1630,"context_line":"            return True"},{"line_number":1631,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_67764907","line":1628,"in_reply_to":"bacf61ea_07ccad87","updated":"2016-08-05 16:01:10.000000000","message":"Yep, exactly.","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"775a992f53bb33c44a5f5118bcf2c8b2a97d74c4","unresolved":false,"context_lines":[{"line_number":1625,"context_line":"            instance \u003d self._lookup_instance(context, instance.uuid)"},{"line_number":1626,"context_line":"            if instance is not None:"},{"line_number":1627,"context_line":"                # If instance is None it has already been deleted."},{"line_number":1628,"context_line":"                instance.destroy()"},{"line_number":1629,"context_line":""},{"line_number":1630,"context_line":"            return True"},{"line_number":1631,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_d621a19f","line":1628,"in_reply_to":"bacf61ea_67764907","updated":"2016-08-05 17:31:53.000000000","message":"I\u0027ll add the exception handling back.","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"584a5da3d8bc75c9bbb6a22fb25e06b1878820bf","unresolved":false,"context_lines":[{"line_number":1625,"context_line":"            instance \u003d self._lookup_instance(context, instance.uuid)"},{"line_number":1626,"context_line":"            if instance is not None:"},{"line_number":1627,"context_line":"                # If instance is None it has already been deleted."},{"line_number":1628,"context_line":"                instance.destroy()"},{"line_number":1629,"context_line":""},{"line_number":1630,"context_line":"            return True"},{"line_number":1631,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_07ccad87","line":1628,"in_reply_to":"bacf61ea_f5a02292","updated":"2016-08-05 15:57:50.000000000","message":"This came up in PS22, I think the point was yeah it\u0027s a race but it\u0027s fine to let it bubble up. Although it seems we\u0027d want to handle InstanceNotFound and return True since it\u0027s gone.","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e83be08dc7be46b272e4c2b46bbf69839f053e30","unresolved":false,"context_lines":[{"line_number":1667,"context_line":"            instance \u003d self._lookup_instance(context, instance.uuid)"},{"line_number":1668,"context_line":"            if not instance:"},{"line_number":1669,"context_line":"                # Instance is already deleted."},{"line_number":1670,"context_line":"                return"},{"line_number":1671,"context_line":""},{"line_number":1672,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":1673,"context_line":"                context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_f5794230","line":1670,"updated":"2016-08-05 15:45:34.000000000","message":"Just to convince myself:\n\n 1. We see the instance is not scheduled, so we will try to delete the BR\n 2. Look up the BR and try to delete and we\u0027re done. If not,\n 3. Look up the instance and delete it. If already gone, we\u0027re done\n 4. If we got here, there is actually an instance record, so do normal delete\n\nRight?\n\nIf we race between 1 and 2, we\u0027re okay because the BR is deleted before the host field is set, so we\u0027ll fail to delete it and know we raced.\n\nIf we race between 2 and 3, we fail to look up the instance record which is already gone and we bail on L1670.\n\nIf we race between 3 and 4, we will do whatever we used to do, which may or may not handle failures properly, but...","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"775a992f53bb33c44a5f5118bcf2c8b2a97d74c4","unresolved":false,"context_lines":[{"line_number":1667,"context_line":"            instance \u003d self._lookup_instance(context, instance.uuid)"},{"line_number":1668,"context_line":"            if not instance:"},{"line_number":1669,"context_line":"                # Instance is already deleted."},{"line_number":1670,"context_line":"                return"},{"line_number":1671,"context_line":""},{"line_number":1672,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":1673,"context_line":"                context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_56b3f14c","line":1670,"in_reply_to":"bacf61ea_27a611ae","updated":"2016-08-05 17:31:53.000000000","message":"Yep, that\u0027s what this is doing.","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"584a5da3d8bc75c9bbb6a22fb25e06b1878820bf","unresolved":false,"context_lines":[{"line_number":1667,"context_line":"            instance \u003d self._lookup_instance(context, instance.uuid)"},{"line_number":1668,"context_line":"            if not instance:"},{"line_number":1669,"context_line":"                # Instance is already deleted."},{"line_number":1670,"context_line":"                return"},{"line_number":1671,"context_line":""},{"line_number":1672,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":1673,"context_line":"                context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_27a611ae","line":1670,"in_reply_to":"bacf61ea_f5794230","updated":"2016-08-05 15:57:50.000000000","message":"Sounds correct to me.","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"}],"nova/compute/cells_api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bde2c9dcb16ea4dcc3273bb0cb870cff74d95094","unresolved":false,"context_lines":[{"line_number":223,"context_line":"            # there isn\u0027t anything to salvage, so we can hard-delete here."},{"line_number":224,"context_line":"            if self._delete_while_booting(context, instance):"},{"line_number":225,"context_line":"                return"},{"line_number":226,"context_line":"            # If instance.host was not set it\u0027s possible that the Instance"},{"line_number":227,"context_line":"            # object here was pulled from a BuildRequest object and is not"},{"line_number":228,"context_line":"            # fully populated. Notably it will be missing an \u0027id\u0027 field which"},{"line_number":229,"context_line":"            # will prevent instance.destroy from functioning properly. A"}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_60bbdac4","line":226,"range":{"start_line":226,"start_character":17,"end_line":226,"end_character":30},"updated":"2016-08-05 13:51:39.000000000","message":"Did you intend to check if not instance.host here?","commit_id":"1c0334e7182a46e24121ef06a3d0f6f313e11113"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"fc4ab3fba0b581d9d5d31f21c5623798d4878ddd","unresolved":false,"context_lines":[{"line_number":223,"context_line":"            # there isn\u0027t anything to salvage, so we can hard-delete here."},{"line_number":224,"context_line":"            if self._delete_while_booting(context, instance):"},{"line_number":225,"context_line":"                return"},{"line_number":226,"context_line":"            # If instance.host was not set it\u0027s possible that the Instance"},{"line_number":227,"context_line":"            # object here was pulled from a BuildRequest object and is not"},{"line_number":228,"context_line":"            # fully populated. Notably it will be missing an \u0027id\u0027 field which"},{"line_number":229,"context_line":"            # will prevent instance.destroy from functioning properly. A"}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_431ce069","line":226,"range":{"start_line":226,"start_character":17,"end_line":226,"end_character":30},"in_reply_to":"bacf61ea_60bbdac4","updated":"2016-08-05 13:59:11.000000000","message":"Copy/paste error. It\u0027s not necessary to check instance.host here because if it were set then instance.cell_name would be set and the check on line 216 would mean we don\u0027t reach this point.","commit_id":"1c0334e7182a46e24121ef06a3d0f6f313e11113"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bde2c9dcb16ea4dcc3273bb0cb870cff74d95094","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            # object here was pulled from a BuildRequest object and is not"},{"line_number":228,"context_line":"            # fully populated. Notably it will be missing an \u0027id\u0027 field which"},{"line_number":229,"context_line":"            # will prevent instance.destroy from functioning properly. A"},{"line_number":230,"context_line":"            # lookup is attempted which will either return a full Instance or"},{"line_number":231,"context_line":"            # will raise InstanceNotFound. If not found then it\u0027s acceptable"},{"line_number":232,"context_line":"            # to skip the rest of the delete processing."},{"line_number":233,"context_line":"            instance \u003d self._lookup_instance(context, instance.uuid)"},{"line_number":234,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_a09af25f","line":231,"range":{"start_line":230,"start_character":75,"end_line":231,"end_character":42},"updated":"2016-08-05 13:51:39.000000000","message":"I don\u0027t think _lookup_instance actually raises InstanceNotFound.","commit_id":"1c0334e7182a46e24121ef06a3d0f6f313e11113"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"fc4ab3fba0b581d9d5d31f21c5623798d4878ddd","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            # object here was pulled from a BuildRequest object and is not"},{"line_number":228,"context_line":"            # fully populated. Notably it will be missing an \u0027id\u0027 field which"},{"line_number":229,"context_line":"            # will prevent instance.destroy from functioning properly. A"},{"line_number":230,"context_line":"            # lookup is attempted which will either return a full Instance or"},{"line_number":231,"context_line":"            # will raise InstanceNotFound. If not found then it\u0027s acceptable"},{"line_number":232,"context_line":"            # to skip the rest of the delete processing."},{"line_number":233,"context_line":"            instance \u003d self._lookup_instance(context, instance.uuid)"},{"line_number":234,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_000f7e37","line":231,"range":{"start_line":230,"start_character":75,"end_line":231,"end_character":42},"in_reply_to":"bacf61ea_a09af25f","updated":"2016-08-05 13:59:11.000000000","message":"Yeah, this is a copy/paste from an older version of the compute/api.py code. I\u0027ll update the comment.","commit_id":"1c0334e7182a46e24121ef06a3d0f6f313e11113"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9177f7888a0cbdd9e20d4ac14656544b1f79caa9","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            # object here was pulled from a BuildRequest object and is not"},{"line_number":228,"context_line":"            # fully populated. Notably it will be missing an \u0027id\u0027 field which"},{"line_number":229,"context_line":"            # will prevent instance.destroy from functioning properly. A"},{"line_number":230,"context_line":"            # lookup is attempted which will either return a full Instance or"},{"line_number":231,"context_line":"            # None if not found. If not found then it\u0027s acceptable to skip the"},{"line_number":232,"context_line":"            # rest of the delete processing."},{"line_number":233,"context_line":"            instance \u003d self._lookup_instance(context, instance.uuid)"},{"line_number":234,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"bacf61ea_18052313","line":231,"range":{"start_line":230,"start_character":75,"end_line":231,"end_character":31},"updated":"2016-08-05 20:03:18.000000000","message":"So if _lookup_instance returns None, don\u0027t you want to skip calling _local_delete because that\u0027s going to blow up trying to access fields on the None instance:\n\nhttps://review.openstack.org/#/c/325985/25/nova/compute/api.py@1892","commit_id":"3774252045f580d671d731b2ab663074f272613d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"74f36fc0c5179c0bb8c5b3580bf0996e52d46bfc","unresolved":false,"context_lines":[{"line_number":217,"context_line":"            delete_type \u003d method_name \u003d\u003d \u0027soft_delete\u0027 and \u0027soft\u0027 or \u0027hard\u0027"},{"line_number":218,"context_line":"            self.cells_rpcapi.instance_delete_everywhere(context,"},{"line_number":219,"context_line":"                    instance, delete_type)"},{"line_number":220,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":221,"context_line":"                    context, instance.uuid)"},{"line_number":222,"context_line":"            # NOTE(danms): If we try to delete an instance with no cell,"},{"line_number":223,"context_line":"            # there isn\u0027t anything to salvage, so we can hard-delete here."}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_26656844","line":220,"updated":"2016-08-05 20:43:06.000000000","message":"nit: follow on optimization but we could move this to where the bdms are actually needed (if we still have an instance to delete).","commit_id":"64e0d33cad6c5610f4f7080aa6626dc9f4f3398f"}],"nova/conductor/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e83be08dc7be46b272e4c2b46bbf69839f053e30","unresolved":false,"context_lines":[{"line_number":515,"context_line":"                # This indicates an instance delete has been requested in the"},{"line_number":516,"context_line":"                # API. Stop the build, cleanup the instance_mapping and"},{"line_number":517,"context_line":"                # potentially the block_device_mappings"},{"line_number":518,"context_line":"                # TODO(alaski): Handle block_device_mapping cleanup"},{"line_number":519,"context_line":"                if inst_mapping:"},{"line_number":520,"context_line":"                    inst_mapping.destroy()"},{"line_number":521,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_1570560a","line":518,"updated":"2016-08-05 15:45:34.000000000","message":"This is updating the comment to do the thing that will be true if you finish your TODO?","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"775a992f53bb33c44a5f5118bcf2c8b2a97d74c4","unresolved":false,"context_lines":[{"line_number":515,"context_line":"                # This indicates an instance delete has been requested in the"},{"line_number":516,"context_line":"                # API. Stop the build, cleanup the instance_mapping and"},{"line_number":517,"context_line":"                # potentially the block_device_mappings"},{"line_number":518,"context_line":"                # TODO(alaski): Handle block_device_mapping cleanup"},{"line_number":519,"context_line":"                if inst_mapping:"},{"line_number":520,"context_line":"                    inst_mapping.destroy()"},{"line_number":521,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_f60dc5e8","line":518,"in_reply_to":"bacf61ea_1570560a","updated":"2016-08-05 17:31:53.000000000","message":"Yes, it was thinking me thinking out loud about where the bdm cleanup should happen. It seems best to handle all cleanup here in the event that we hit this stop build marker.","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"}],"nova/tests/fixtures.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8e4fe7a30ec145d4a41ef837905a93771ea07675","unresolved":false,"context_lines":[{"line_number":761,"context_line":"        return _noop_rpc"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":""},{"line_number":764,"context_line":"class NoopConductorFixture(fixtures.Fixture):"},{"line_number":765,"context_line":"    \"\"\"Stub out the conductor API to do nothing\"\"\""},{"line_number":766,"context_line":""},{"line_number":767,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_a3504d33","line":764,"updated":"2016-07-20 20:08:27.000000000","message":"Coolio, thanks for genericizing this :)","commit_id":"abbb2ecdb1becaad02006dc2f1388e6c5fd7dc05"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d5dc79af28d7958f26148d200fbb6109d230b8a2","unresolved":false,"context_lines":[{"line_number":806,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":807,"context_line":"            \u0027nova.conductor.ComputeTaskApi\u0027, _NoopConductor))"},{"line_number":808,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":809,"context_line":"            \u0027nova.conductor.API\u0027, _NoopConductor))"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_754f1292","line":809,"updated":"2016-08-05 15:46:05.000000000","message":"You need tests for these. I would recommend you split these out, add tests, and we land that in a patch before this one.","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"775a992f53bb33c44a5f5118bcf2c8b2a97d74c4","unresolved":false,"context_lines":[{"line_number":806,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":807,"context_line":"            \u0027nova.conductor.ComputeTaskApi\u0027, _NoopConductor))"},{"line_number":808,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":809,"context_line":"            \u0027nova.conductor.API\u0027, _NoopConductor))"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_760275b5","line":809,"in_reply_to":"bacf61ea_754f1292","updated":"2016-08-05 17:31:53.000000000","message":"Can do.","commit_id":"98b3b65a010ddafd51124a5ace0d55cd179a7b00"}],"nova/tests/functional/wsgi/test_servers.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0d96ff5fb81ec430b8e65b2b6da2e02d4d9713cf","unresolved":false,"context_lines":[{"line_number":14,"context_line":"from nova.tests import fixtures as nova_fixtures"},{"line_number":15,"context_line":"from nova.tests.unit.image import fake as fake_image"},{"line_number":16,"context_line":"from nova.tests.unit import policy_fixture"},{"line_number":17,"context_line":"# from nova.tests import uuidsentinel as uuids"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"class NoopConductor(object):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_4d82b995","line":17,"updated":"2016-06-10 19:26:15.000000000","message":"Probably want to remove this.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"daa6d784f64736a74eee8f4bb01efd5b67cc5231","unresolved":false,"context_lines":[{"line_number":14,"context_line":"from nova.tests import fixtures as nova_fixtures"},{"line_number":15,"context_line":"from nova.tests.unit.image import fake as fake_image"},{"line_number":16,"context_line":"from nova.tests.unit import policy_fixture"},{"line_number":17,"context_line":"# from nova.tests import uuidsentinel as uuids"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"class NoopConductor(object):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_b33e1ed8","line":17,"in_reply_to":"7aa08908_4d82b995","updated":"2016-06-10 20:01:27.000000000","message":"woops, holdover from when there was another test in here which I think I add later.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5da42f442fbb718f381179be77b735c98c478191","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    \"\"\"Tests for the servers API with unscheduled instances."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    With cellsv2 an instance is not written to an instance table in the"},{"line_number":31,"context_line":"    database until it has been scheduled to a cell. This means we need to be"},{"line_number":32,"context_line":"    careful to ensure the instance can still be represented before that point."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    This test class starts the wsgi stack for the nova api service, and uses"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_787e316b","line":31,"updated":"2016-06-13 00:48:48.000000000","message":"s/database/cell database/ (just to be explicitly clear)","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d61850327aa508e07e72b80d380859509963a614","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    \"\"\"Tests for the servers API with unscheduled instances."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    With cellsv2 an instance is not written to an instance table in the"},{"line_number":31,"context_line":"    database until it has been scheduled to a cell. This means we need to be"},{"line_number":32,"context_line":"    careful to ensure the instance can still be represented before that point."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    This test class starts the wsgi stack for the nova api service, and uses"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_f83fa8b1","line":31,"in_reply_to":"7aa08908_787e316b","updated":"2016-06-13 14:22:37.000000000","message":"Can do.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0d96ff5fb81ec430b8e65b2b6da2e02d4d9713cf","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        super(ServersPreSchedulingTestCase, self).setUp()"},{"line_number":42,"context_line":"        fake_image.stub_out_image_service(self)"},{"line_number":43,"context_line":"        self.useFixture(policy_fixture.RealPolicyFixture())"},{"line_number":44,"context_line":"        self.stub_out(\u0027nova.conductor.ComputeTaskAPI\u0027, NoopConductor)"},{"line_number":45,"context_line":"        api_fixture \u003d self.useFixture(nova_fixtures.OSAPIFixture("},{"line_number":46,"context_line":"            api_version\u003d\u0027v2.1\u0027))"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_303064e8","line":44,"updated":"2016-06-10 19:26:15.000000000","message":"So this is what prevents it from getting scheduled?","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"daa6d784f64736a74eee8f4bb01efd5b67cc5231","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        super(ServersPreSchedulingTestCase, self).setUp()"},{"line_number":42,"context_line":"        fake_image.stub_out_image_service(self)"},{"line_number":43,"context_line":"        self.useFixture(policy_fixture.RealPolicyFixture())"},{"line_number":44,"context_line":"        self.stub_out(\u0027nova.conductor.ComputeTaskAPI\u0027, NoopConductor)"},{"line_number":45,"context_line":"        api_fixture \u003d self.useFixture(nova_fixtures.OSAPIFixture("},{"line_number":46,"context_line":"            api_version\u003d\u0027v2.1\u0027))"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_de09312e","line":44,"in_reply_to":"7aa08908_303064e8","updated":"2016-06-10 20:01:27.000000000","message":"Yep. It goes to the point where it casts to the conductor to schedule it and then gets nooped.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0d96ff5fb81ec430b8e65b2b6da2e02d4d9713cf","unresolved":false,"context_lines":[{"line_number":58,"context_line":"            }"},{"line_number":59,"context_line":"        }"},{"line_number":60,"context_line":"        create_resp \u003d self.api.api_post(\u0027servers\u0027, body)"},{"line_number":61,"context_line":"        self.api.api_delete(\u0027servers/%s\u0027 % create_resp.body[\u0027server\u0027][\u0027id\u0027])"},{"line_number":62,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":63,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027],"},{"line_number":64,"context_line":"                                    check_response_status\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_d0a5a004","line":61,"updated":"2016-06-10 19:26:15.000000000","message":"Would it be worth asserting that the BuildRequest exists before calling delete?","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"daa6d784f64736a74eee8f4bb01efd5b67cc5231","unresolved":false,"context_lines":[{"line_number":58,"context_line":"            }"},{"line_number":59,"context_line":"        }"},{"line_number":60,"context_line":"        create_resp \u003d self.api.api_post(\u0027servers\u0027, body)"},{"line_number":61,"context_line":"        self.api.api_delete(\u0027servers/%s\u0027 % create_resp.body[\u0027server\u0027][\u0027id\u0027])"},{"line_number":62,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":63,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027],"},{"line_number":64,"context_line":"                                    check_response_status\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_9e37a9af","line":61,"in_reply_to":"7aa08908_d0a5a004","updated":"2016-06-10 20:01:27.000000000","message":"I could, but I add another test in a later patch which does the post/get check. So that case will be covered. In this patch it\u0027s not actually pulling from the BuildRequest yet, the delete path immediately falls back to the regular delete. So the test is a little synthetic. It\u0027s here so that a patch or two after this the test is already in place when deletes do start to use the BuildRequest.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5da42f442fbb718f381179be77b735c98c478191","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":63,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027],"},{"line_number":64,"context_line":"                                    check_response_status\u003dFalse)"},{"line_number":65,"context_line":"        self.assertEqual(404, get_resp.status)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_d897658e","line":65,"updated":"2016-06-13 00:48:48.000000000","message":"What was the response code returned *before* this patch?","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d61850327aa508e07e72b80d380859509963a614","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":63,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027],"},{"line_number":64,"context_line":"                                    check_response_status\u003dFalse)"},{"line_number":65,"context_line":"        self.assertEqual(404, get_resp.status)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_18b1d447","line":65,"in_reply_to":"7aa08908_d897658e","updated":"2016-06-13 14:22:37.000000000","message":"The same. This patch doesn\u0027t change it, it\u0027s attempting to keep the response the same when delete request is received for an instance that is not in the db, the BuildRequest is.","commit_id":"59bade8c9d9fee510b9f42ea48afdce06ba8a345"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e639d36deadcb73ded4193e13e8cb7e8d92abf07","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from nova.tests.unit import policy_fixture"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class NoopConductor(object):"},{"line_number":20,"context_line":"    def __getattr__(self, key):"},{"line_number":21,"context_line":"        def _noop_rpc(*args, **kwargs):"},{"line_number":22,"context_line":"            return None"}],"source_content_type":"text/x-python","patch_set":17,"id":"dada55a8_220e603b","line":19,"updated":"2016-07-19 16:28:33.000000000","message":"femtonit... might be good to make this a generic functional test fixture in a future patch.","commit_id":"e909bc7252648859e09319b0b0bc57a11e942418"}]}