Revert "Overhaul bash-completion to support non-UUID based IDs"

This reverts commit 4c8cefb98a.

The cache completion was causing a bug where 'nova volume-attach' would
then try to query Nova for volume information using a URL that was not
valid.  This caused it to appear that the attach had failed.

Additionally the idea of making extra API queries for a bash completion
cache should be thought through more.

Conflicts:

	novaclient/client.py
	novaclient/shell.py
	novaclient/v2/client.py
	novaclient/v3/client.py

Closes-Bug: #1423695
Change-Id: I6676a11f9b5318a1cda2d03a5d4550bda549d5c2

novaclient/base.py

@@ -20,12 +20,17 @@ Base utilities to build API operation managers and objects on top of.
 """
 
 import abc
+import contextlib
+import hashlib
 import inspect
+import os
+import threading
 
 import six
 
 from novaclient import exceptions
 from novaclient.openstack.common.apiclient import base
+from novaclient.openstack.common import cliutils
 
 Resource = base.Resource
 
@@ -47,18 +52,11 @@ class Manager(base.HookableMixin):
     etc.) and provide CRUD operations for them.
     """
     resource_class = None
+    cache_lock = threading.RLock()
 
     def __init__(self, api):
         self.api = api
 
-    def _write_object_to_completion_cache(self, obj):
-        if hasattr(self.api, 'write_object_to_completion_cache'):
-            self.api.write_object_to_completion_cache(obj)
-
-    def _clear_completion_cache_for_class(self, obj_class):
-        if hasattr(self.api, 'clear_completion_cache_for_class'):
-            self.api.clear_completion_cache_for_class(obj_class)
-
     def _list(self, url, response_key, obj_class=None, body=None):
         if body:
             _resp, body = self.api.client.post(url, body=body)
@@ -77,22 +75,77 @@ class Manager(base.HookableMixin):
             except KeyError:
                 pass
 
-        self._clear_completion_cache_for_class(obj_class)
+        with self.completion_cache('human_id', obj_class, mode="w"):
+            with self.completion_cache('uuid', obj_class, mode="w"):
+                return [obj_class(self, res, loaded=True)
+                        for res in data if res]
 
-        objs = []
+    @contextlib.contextmanager
-        for res in data:
+    def completion_cache(self, cache_type, obj_class, mode):
-            if res:
+        """
-                obj = obj_class(self, res, loaded=True)
+        The completion cache store items that can be used for bash
-                self._write_object_to_completion_cache(obj)
+        autocompletion, like UUIDs or human-friendly IDs.
-                objs.append(obj)
 
-        return objs
+        A resource listing will clear and repopulate the cache.
+
+        A resource create will append to the cache.
+
+        Delete is not handled because listings are assumed to be performed
+        often enough to keep the cache reasonably up-to-date.
+        """
+        # NOTE(wryan): This lock protects read and write access to the
+        # completion caches
+        with self.cache_lock:
+            base_dir = cliutils.env('NOVACLIENT_UUID_CACHE_DIR',
+                                    default="~/.novaclient")
+
+            # NOTE(sirp): Keep separate UUID caches for each username +
+            # endpoint pair
+            username = cliutils.env('OS_USERNAME', 'NOVA_USERNAME')
+            url = cliutils.env('OS_URL', 'NOVA_URL')
+            uniqifier = hashlib.md5(username.encode('utf-8') +
+                                    url.encode('utf-8')).hexdigest()
+
+            cache_dir = os.path.expanduser(os.path.join(base_dir, uniqifier))
+
+            try:
+                os.makedirs(cache_dir, 0o755)
+            except OSError:
+                # NOTE(kiall): This is typically either permission denied while
+                #              attempting to create the directory, or the
+                #              directory already exists. Either way, don't
+                #              fail.
+                pass
+
+            resource = obj_class.__name__.lower()
+            filename = "%s-%s-cache" % (resource, cache_type.replace('_', '-'))
+            path = os.path.join(cache_dir, filename)
+
+            cache_attr = "_%s_cache" % cache_type
+
+            try:
+                setattr(self, cache_attr, open(path, mode))
+            except IOError:
+                # NOTE(kiall): This is typically a permission denied while
+                #              attempting to write the cache file.
+                pass
+
+            try:
+                yield
+            finally:
+                cache = getattr(self, cache_attr, None)
+                if cache:
+                    cache.close()
+                    delattr(self, cache_attr)
+
+    def write_to_completion_cache(self, cache_type, val):
+        cache = getattr(self, "_%s_cache" % cache_type, None)
+        if cache:
+            cache.write("%s\n" % val)
 
     def _get(self, url, response_key):
         _resp, body = self.api.client.get(url)
-        obj = self.resource_class(self, body[response_key], loaded=True)
+        return self.resource_class(self, body[response_key], loaded=True)
-        self._write_object_to_completion_cache(obj)
-        return obj
 
     def _create(self, url, body, response_key, return_raw=False, **kwargs):
         self.run_hooks('modify_body_for_create', body, **kwargs)
@@ -100,9 +153,9 @@ class Manager(base.HookableMixin):
         if return_raw:
             return body[response_key]
 
-        obj = self.resource_class(self, body[response_key])
+        with self.completion_cache('human_id', self.resource_class, mode="a"):
-        self._write_object_to_completion_cache(obj)
+            with self.completion_cache('uuid', self.resource_class, mode="a"):
-        return obj
+                return self.resource_class(self, body[response_key])
 
     def _delete(self, url):
         _resp, _body = self.api.client.delete(url)

novaclient/client.py

@@ -21,12 +21,9 @@ OpenStack Client interface. Handles the REST calls and responses.
 """
 
 import copy
-import errno
 import functools
-import glob
 import hashlib
 import logging
-import os
 import re
 import socket
 import time
@@ -46,7 +43,6 @@ from six.moves.urllib import parse
 
 from novaclient import exceptions
 from novaclient.i18n import _
-from novaclient.openstack.common import cliutils
 from novaclient import service_catalog
 
 
@@ -76,77 +72,6 @@ class _ClientConnectionPool(object):
         return self._adapters[url]
 
 
-class CompletionCache(object):
-    """The completion cache is how we support tab-completion with novaclient.
-
-    The `Manager` writes object IDs and Human-IDs to the completion-cache on
-    object-show, object-list, and object-create calls.
-
-    The `nova.bash_completion` script then uses these files to provide the
-    actual tab-completion.
-
-    The cache directory layout is:
-
-        ~/.novaclient/
-            <hash-of-endpoint-and-username>/
-                <resource>-id-cache
-                <resource>-name-cache
-    """
-    def __init__(self, username, auth_url, attributes=('id', 'name')):
-        self.directory = self._make_directory_name(username, auth_url)
-        self.attributes = attributes
-
-    def _make_directory_name(self, username, auth_url):
-        """Creates a unique directory name based on the auth_url and username
-        of the current user.
-        """
-        uniqifier = hashlib.md5(username.encode('utf-8') +
-                                auth_url.encode('utf-8')).hexdigest()
-        base_dir = cliutils.env('NOVACLIENT_UUID_CACHE_DIR',
-                                default="~/.novaclient")
-        return os.path.expanduser(os.path.join(base_dir, uniqifier))
-
-    def _prepare_directory(self):
-        try:
-            os.makedirs(self.directory, 0o755)
-        except OSError:
-            # NOTE(kiall): This is typically either permission denied while
-            #              attempting to create the directory, or the
-            #              directory already exists. Either way, don't
-            #              fail.
-            pass
-
-    def clear_class(self, obj_class):
-        self._prepare_directory()
-
-        resource = obj_class.__name__.lower()
-        resource_glob = os.path.join(self.directory, "%s-*-cache" % resource)
-
-        for filename in glob.iglob(resource_glob):
-            try:
-                os.unlink(filename)
-            except OSError as e:
-                if e.errno != errno.ENOENT:
-                    raise
-
-    def _write_attribute(self, resource, attribute, value):
-        self._prepare_directory()
-
-        filename = "%s-%s-cache" % (resource, attribute.replace('_', '-'))
-        path = os.path.join(self.directory, filename)
-
-        with open(path, 'a') as f:
-            f.write("%s\n" % value)
-
-    def write_object(self, obj):
-        resource = obj.__class__.__name__.lower()
-
-        for attribute in self.attributes:
-            value = getattr(obj, attribute, None)
-            if value:
-                self._write_attribute(resource, attribute, value)
-
-
 class SessionClient(adapter.LegacyJsonAdapter):
 
     def __init__(self, *args, **kwargs):

novaclient/shell.py

@@ -748,8 +748,6 @@ class OpenStackComputeShell(object):
                     _("You must provide an auth url "
                       "via either --os-auth-url or env[OS_AUTH_URL]"))
 
-        completion_cache = client.CompletionCache(os_username, os_auth_url)
-
         self.cs = client.Client(
             options.os_compute_api_version,
             os_username, os_password, os_tenant_name,
@@ -763,8 +761,7 @@ class OpenStackComputeShell(object):
             timings=args.timings, bypass_url=bypass_url,
             os_cache=os_cache, http_log_debug=options.debug,
             cacert=cacert, timeout=timeout,
-            session=keystone_session, auth=keystone_auth,
+            session=keystone_session, auth=keystone_auth)
-            completion_cache=completion_cache)
 
         # Now check for the password/token of which pieces of the
         # identifying keyring key can come from the underlying client

novaclient/tests/unit/test_client.py

@@ -16,7 +16,6 @@
 
 import json
 import logging
-import os
 
 import fixtures
 import mock
@@ -369,31 +368,3 @@ class ClientTest(utils.TestCase):
         self.assertIn('RESP BODY: {"access": {"token": {"id":'
                       ' "{SHA1}4fc49c6a671ce889078ff6b250f7066cf6d2ada2"}}}',
                       output)
-
-    @mock.patch('novaclient.client.HTTPClient')
-    def test_completion_cache(self, instance):
-        cp_cache = novaclient.client.CompletionCache('user',
-                                                     "server/v2")
-
-        client = novaclient.v2.client.Client("user",
-                                             "password", "project_id",
-                                             auth_url="server/v2",
-                                             completion_cache=cp_cache)
-
-        instance.id = "v1c49c6a671ce889078ff6b250f7066cf6d2ada2"
-        instance.name = "foo.bar.baz v1_1"
-        client.write_object_to_completion_cache(instance)
-        self.assertTrue(os.path.isdir(cp_cache.directory))
-
-        for file_name in os.listdir(cp_cache.directory):
-            file_path = os.path.join(cp_cache.directory, file_name)
-            f = open(file_path, 'r')
-            for line in f:
-                line = line.rstrip()
-                if '-id-' in file_name:
-                    self.assertEqual(instance.id, line)
-                elif '-name-' in file_name:
-                    self.assertEqual(instance.name, line)
-            f.close()
-            os.remove(file_path)
-        os.rmdir(cp_cache.directory)

novaclient/v2/client.py

@@ -103,7 +103,7 @@ class Client(object):
                  auth_system='keystone', auth_plugin=None, auth_token=None,
                  cacert=None, tenant_id=None, user_id=None,
                  connection_pool=False, session=None, auth=None,
-                 completion_cache=None, **kwargs):
+                 **kwargs):
         # FIXME(comstud): Rename the api_key argument above when we
         # know it's not being used as keyword argument
 
@@ -196,16 +196,6 @@ class Client(object):
             auth=auth,
             **kwargs)
 
-        self.completion_cache = completion_cache
-
-    def write_object_to_completion_cache(self, obj):
-        if self.completion_cache:
-            self.completion_cache.write_object(obj)
-
-    def clear_completion_cache_for_class(self, obj_class):
-        if self.completion_cache:
-            self.completion_cache.clear_class(obj_class)
-
     @client._original_only
     def __enter__(self):
         self.client.open_session()