Fix code review comments from David Foster.

Subclass ValueError into AmbiguousReference for cases when there are multiple entities in a collection with the same name. Comment on a few tests that are weird. Simplify __contains__ to refer to __getitem__, which now handles all the necessary cases.

Author: Frederick Ross

splunklib/client.py

@@ -113,6 +113,9 @@ class IncomparableException(Exception):
 class JobNotReadyException(Exception):
     pass
 
+class AmbiguousReference(ValueError):
+    pass
+
 def trailing(template, *targets):
     """Substring of *template* following all *targets*.
 
@@ -886,19 +889,12 @@ def __contains__(self, name):
         if autologin is enabled.
         """
         try:
-            response = self.get(name)
-            entries = self._load_list(response)
-            if len(entries) == 0:
-                # We need this because in 4.2 and 4.3,
-                # fired alerts return an empty feed instead of 404.
-                return False 
-            else:
-                return True
-        except HTTPError as he:
-            if he.status == 404:
-                return False
-            else:
-                raise
+            self[name]
+            return True
+        except KeyError:
+            return False
+        except AmbiguousReference:
+            return True
 
     def __getitem__(self, key):
         """Fetch an item named *key* from this collection.
@@ -958,7 +954,7 @@ def __getitem__(self, key):
             response = self.get(key, owner=ns.owner, app=ns.app)
             entries = self._load_list(response)
             if len(entries) > 1:
-                raise ValueError("Found multiple entities named '%s'; please specify a namespace." % key)
+                raise AmbiguousReference("Found multiple entities named '%s'; please specify a namespace." % key)
             elif len(entries) == 0:
                 raise KeyError(key)
             else:
@@ -1013,7 +1009,10 @@ def _entity_path(self, state):
         """Calculate the path to an entity to be returned.
 
         *state* should be the dictionary returned by
-        :func:`_parse_atom_entry`.
+        :func:`_parse_atom_entry`. :func:`_entity_path` extracts the
+        link to this entity from *state*, and strips all the namespace
+        prefixes from it to leave only the relative path of the entity
+        itself, sans namespace.
 
         :rtype: string
         :returns: an absolute path
@@ -1522,6 +1521,8 @@ def __contains__(self, key):
             return True
         except KeyError:
             return False
+        except AmbiguousReference:
+            return True
 
 
     def create(self, kind, name, **kwargs):

tests/test_fired_alert.py

@@ -94,9 +94,9 @@ def f(search):
         testlib.wait(search, f, timeout=120)
         self.assertEqual(len(search.history()), 1)
 
-        # When it first runs the alert count should be zero.
+        # There should be no alerts if the search job hasn't been
+        # created yet.
         search.refresh()
-        print alert_count(search)
         self.assertTrue((alert_count(search) == 0) == (search_name not in fired_alerts))
 
         # Submit events and verify that they each trigger the expected

tests/test_job.py

@@ -182,7 +182,7 @@ def test_results(self):
 
         result = reader.next()
         self.assertTrue(isinstance(result, dict))
-        self.assertLessEqual(int(result["count"]), 1000)
+        self.assertLessEqual(int(result["count"]), 1)
 
         # Repeat the same thing, but without the .is_preview reference.
         job = jobs.create("search index=_internal | head 1 | stats count")
@@ -193,7 +193,7 @@ def test_results(self):
         self.assertEqual(job['isDone'], '1')
         result = reader.next()
         self.assertTrue(isinstance(result, dict))
-        self.assertLessEqual(int(result["count"]), 100)
+        self.assertLessEqual(int(result["count"]), 1)
 
     def test_results_reader(self):
         # Run jobs.export("search index=_internal | stats count",

tests/test_saved_search.py

@@ -199,7 +199,8 @@ def test_dispatch(self):
         try:
             job.preview().close()
         except ValueError:
-            pass
+            pass # Probably not in a state that we can actually get
+                 # events from yet.
         job.cancel()
 
         # Dispatch with some additional options