)]}'
{"ironic/nova/virt/ironic/driver.py":[{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"aee60cf6ffa47d21fc2965b012e51d42e7ab6d3c","unresolved":false,"context_lines":[{"line_number":303,"context_line":"    def get_hypervisor_version(self):"},{"line_number":304,"context_line":"        return CONF.ironic.api_version"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"    def list_instances(self):"},{"line_number":307,"context_line":"        icli \u003d client_wrapper.IronicClientWrapper()"},{"line_number":308,"context_line":"        node_list \u003d icli.call(\"node.list\")"},{"line_number":309,"context_line":"        instances \u003d [i for i in node_list if i.instance_uuid]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FcKA%3D","side":"PARENT","line":306,"updated":"2014-04-21 19:16:51.000000000","message":"FWIW, this method is broken. It should return a list of instance.name.  However, that would be very expensive.\n\nThis method would never be called if we implement \u0027instance_exists\u0027 and \u0027list_instance_uuids\u0027. So, after my fix to nova, I propose we add list_instance_uuids() here and drop this method.","commit_id":"9ff949c8c16a89998d1fd1271ff16a8836363a9e"},{"author":{"_account_id":1420,"name":"Adam Gandelman","email":"adamg@ubuntu.com","username":"gandelman-a"},"change_message_id":"a5347e8acc185cbd7ac9f9fbb3bbca1f345023e8","unresolved":false,"context_lines":[{"line_number":303,"context_line":"    def get_hypervisor_version(self):"},{"line_number":304,"context_line":"        return CONF.ironic.api_version"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"    def list_instances(self):"},{"line_number":307,"context_line":"        icli \u003d client_wrapper.IronicClientWrapper()"},{"line_number":308,"context_line":"        node_list \u003d icli.call(\"node.list\")"},{"line_number":309,"context_line":"        instances \u003d [i for i in node_list if i.instance_uuid]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FcAU%3D","side":"PARENT","line":306,"in_reply_to":"AAAAXH%2F%2FcKA%3D","updated":"2014-04-21 19:47:33.000000000","message":"list_instance_uuids() patch proposed here: https://review.openstack.org/#/c/88684/. There seems to be a lot of wasteful accounting going on in the nova driver wrt instance name that doesn\u0027t really apply to ironic.","commit_id":"9ff949c8c16a89998d1fd1271ff16a8836363a9e"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"aee60cf6ffa47d21fc2965b012e51d42e7ab6d3c","unresolved":false,"context_lines":[{"line_number":305,"context_line":"    def get_hypervisor_version(self):"},{"line_number":306,"context_line":"        return CONF.ironic.api_version"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    def instance_exists(self, instance_name):"},{"line_number":309,"context_line":"        \"\"\"Checks the existence of an instance."},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"        Checks the existence of an instance. This is an override of the"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FcJ0%3D","line":308,"updated":"2014-04-21 19:16:51.000000000","message":"I\u0027m making a patch to nova that passes \u0027instance\u0027 here instead of instance_name. That means the logic below could just do the single node.get_by_instance_uuid, instance[\u0027uuid\u0027]","commit_id":"3d5176620587961ff644385dafa5fad4c79ac52a"},{"author":{"_account_id":1420,"name":"Adam Gandelman","email":"adamg@ubuntu.com","username":"gandelman-a"},"change_message_id":"5f49c03ad932a66638cd646d97e04ddd6407757f","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        # get_by_name(), implement it."},{"line_number":322,"context_line":"        filters \u003d {\u0027name\u0027: instance_name}"},{"line_number":323,"context_line":"        instances \u003d instance_obj.InstanceList.get_by_filters(ctxt, filters)"},{"line_number":324,"context_line":"        instance_uuid \u003d [i.uuid for i in instances][0]"},{"line_number":325,"context_line":"        try:"},{"line_number":326,"context_line":"            icli.call(\"node.get_by_instance_uuid\", instance_uuid)"},{"line_number":327,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2Fnf0%3D","line":324,"updated":"2014-04-18 19:23:46.000000000","message":"Should this first check instances is not empty, and return False if so, to prevent a possible KeyError on instances[0]?","commit_id":"3d5176620587961ff644385dafa5fad4c79ac52a"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"cbf6821a2070592be0076e682146cd90c0f62012","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        # get_by_name(), implement it."},{"line_number":322,"context_line":"        filters \u003d {\u0027name\u0027: instance_name}"},{"line_number":323,"context_line":"        instances \u003d instance_obj.InstanceList.get_by_filters(ctxt, filters)"},{"line_number":324,"context_line":"        instance_uuid \u003d [i.uuid for i in instances][0]"},{"line_number":325,"context_line":"        try:"},{"line_number":326,"context_line":"            icli.call(\"node.get_by_instance_uuid\", instance_uuid)"},{"line_number":327,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2Fmbk%3D","line":324,"in_reply_to":"AAAAXH%2F%2Fnf0%3D","updated":"2014-04-18 22:38:42.000000000","message":"This is called by nova.compute.manager, upon its unwrapping of the instance object. So just reversing that to create another instance object from Nova is really defeating the purpose of this check:\n\n  1508     def _check_instance_exists(self, context, instance):\n  1509         \"\"\"Ensure an instance with the same name is not already present.\"\"\"\n  1510         if self.driver.instance_exists(instance[\u0027name\u0027]):\n  1511             raise exception.InstanceExists(name\u003dinstance[\u0027name\u0027])\n  1512 \n\nIronic does not require the instance \"name\" to be unique, and in fact, isn\u0027t storing it anywhere at all, which is the source of this confusion.\n\nIMBW, but I believe the intent of this method is to ensure that, for local hypervisors which require unique instance names, duplication is prevented. It doesn\u0027t make sense for a federated hypervisor to enforce any uniqueness beyond the instance_uuid itself. So, we should just patch Nova to fix this assumption.","commit_id":"3d5176620587961ff644385dafa5fad4c79ac52a"}]}