Resolved comments in code reviews by Itay and David.

Author: Frederick Ross

docs/tutorial.rst

@@ -1,7 +1,11 @@
 Introduction
 ------------
 
-Here's a simple program using the Python SDK. Obviously you'll have to change the host, username, password, and any other data that you may have customized. And don't experiment on your production Splunk server! Install the free version of Splunk on your own machine to experiment.::
+Here's a simple program using the Python SDK. Obviously you'll have to
+change the host, username, password, and any other data that you may
+have customized. And don't experiment on your production Splunk
+server! Install the free version of Splunk on your own machine to
+experiment.::
 
     import splunklib.client as client
     c = client.connect(host="localhost",
@@ -14,31 +18,61 @@ Here's a simple program using the Python SDK. Obviously you'll have to change th
     assert "my_saved_search" in saved_searches
     saved_searches.delete("my_saved_search")
 
-It's worth spending a few minute in ``ipython`` examining the objects produced in this example. ``c`` is a ``Service``[[TODO: link to reference docs]], which has [fields](link to list of fields in Service's docs) that provide access to most of Splunk's contents. ``saved_searches`` is a ``Collection``, and each entity in it is identified by a unique name (``"my_saved_search"`` in the example). All the names should be alphanumeric plus ``_`` and ``-``; no spaces are allowed[#]_.
-
-.. [#] Splunk has two names for each entity: the pretty one meant to be displayed to users in the web browser, and the alphanumeric one that shows up in the URL of the REST call. It is the latter that is used in the SDK. Thus the Search app in Splunk is called "Search" in the web interface, but to fetch it via the SDK, you would write ``c.apps['search']``, not ``c.apps['Search']``. The "Getting Started" app is ``c.apps['gettingstarted']``, not ``c.apps['Getting Started']``.
-
-A ``Collection`` acts like a dictionary. You can call ``keys``, ``iteritems``, and ``itervalues`` just like on a dictionary. However, you cannot assign to keys. ``saved_searches['some_name'] = ...`` is nonsense. Use the ``create`` method instead. Also, ``del saved_searches['some_name']`` does not currently work. Use the ``delete`` method instead.
+It's worth spending a few minute in ``ipython`` examining the objects
+produced in this example. ``c`` is a ``Service``[[TODO: link to
+reference docs]], which has [fields](link to list of fields in
+Service's docs) that provide access to most of Splunk's contents.
+``saved_searches`` is a ``Collection``, and each entity in it is
+identified by a unique name (``"my_saved_search"`` in the example).
+All the names should be alphanumeric plus ``_`` and ``-``; no spaces
+are allowed[#]_.
+
+.. [#] Splunk has two names for each entity: the pretty one meant to
+be displayed to users in the web browser, and the alphanumeric one
+that shows up in the URL of the REST call. It is the latter that is
+used in the SDK. Thus the Search app in Splunk is called "Search"
+in the web interface, but to fetch it via the SDK, you would write
+``c.apps['search']``, not ``c.apps['Search']``. The "Getting
+Started" app is ``c.apps['gettingstarted']``, not ``c.apps['Getting Started']``.
+
+A ``Collection`` acts like a dictionary. You can call ``keys``,
+``iteritems``, and ``itervalues`` just like on a dictionary. However,
+you cannot assign to keys. ``saved_searches['some_name'] = ...`` is
+nonsense. Use the ``create`` method instead. Also, 
+``del saved_searches['some_name']`` does not currently work. Use the
+``delete`` method instead.
 
 Note that in the example code we did not assert::
 
     mss == saved_searches["my_saved_search"]
 
-The Python objects you are manipulating represent snapshots of the server's state at some point in the past. There is no good way of defining equality on these that isn't misleading in many cases, so we have made ``==`` and ``!=`` raise exceptions for entities. 
+The Python objects you are manipulating represent snapshots of the
+server's state at some point in the past. There is no good way of
+defining equality on these that isn't misleading in many cases, so we
+have made ``==`` and ``!=`` raise exceptions for entities.
 
-Another side effect of using snapshots: after we delete the saved search in the example, ``mss`` is still bound to the same local object representing that search, even though it no longer exists on the server. If you need to update your snapshot, call the ``refresh`` method[#]_. For more on caching and snapshots, see [[TODO: link to section on roundtrips and caching]]
+Another side effect of using snapshots: after we delete the saved
+search in the example, ``mss`` is still bound to the same local object
+representing that search, even though it no longer exists on the
+server. If you need to update your snapshot, call the ``refresh``
+method[#]_. For more on caching and snapshots, see [[TODO: link to
+section on roundtrips and caching]]
 
-.. [#] Calling ``refresh`` on an entity that has already been deleted raises an ``HTTPError``.
+.. [#] Calling ``refresh`` on an entity that has already been deleted
+raises an ``HTTPError``.
 
-You can access the fields of an entity either as if they were keys in a dictionary, or fields of an object::
+You can access the fields of an entity either as if they were keys in
+a dictionary, or fields of an object::
 
     mss['search'] == "search * | head 10"
     mss.search    == "search * | head 10"
 
     mss['action.email'] == '0'
     mss.action.email    == '0'
 
-A ``.`` isn't a valid character in identifiers in Python. The second form is actually a series of field lookups. As as side effect, you can get groups of fields that share prefixes.::
+A ``.`` isn't a valid character in identifiers in Python. The second
+form is actually a series of field lookups. As as side effect, you can
+get groups of fields that share prefixes.::
 
     mss['action'] == {'email': '0',
                       'populate_lookup': '0',
@@ -51,17 +85,27 @@ A ``.`` isn't a valid character in identifiers in Python. The second form is act
                    'script': '0',
                    'summary_index': '0'}
 
-Those look like dictionaries, but they're actually a subclass called ``Record`` [[TODO: link to reference documentation]] that allows keys to be looked up as fields. [[TODO: Implement keys() on entities, and document it here]] In addition to fields, each kind of entity has a range of methods.::
+Those look like dictionaries, but they're actually a subclass called
+``Record`` [[TODO: link to reference documentation]] that allows keys
+to be looked up as fields. [[TODO: Implement keys() on entities, and
+document it here]] In addition to fields, each kind of entity has a
+range of methods.::
 
     mss.dispatch()    # Runs the saved search.
     mss.suppress(30)  # Suppress all alerts from this saved search for 30 seconds
 
-This should be enough information to understand the reference documentation and start using the SDK productively.
+This should be enough information to understand the reference
+documentation and start using the SDK productively.
 
 Roundtrips and caching
 ----------------------
 
-The rate limiting step in most programs that call REST APIs is calls to the server. The SDK is designed to minimize and postpone these as much as possible. When you fetch an object from the SDK, you get a snapshot. If there are updates on the server after that snapshot, you won't know about them until you call ``refresh`` on your object. The object might even have been deleted.
+The rate limiting step in most programs that call REST APIs is calls
+to the server. The SDK is designed to minimize and postpone these as
+much as possible. When you fetch an object from the SDK, you get a
+snapshot. If there are updates on the server after that snapshot, you
+won't know about them until you call ``refresh`` on your object. The
+object might even have been deleted.
 
 
 

setup.py

@@ -25,6 +25,8 @@
 
     description="The Splunk Software Development Kit for Python.",
 
+    install_requires=['ordereddict'],
+
     license="http://www.apache.org/licenses/LICENSE-2.0",
 
     name="splunk-sdk",

splunklib/binding.py

@@ -30,6 +30,7 @@
 import socket
 import ssl
 import urllib
+import functools
 
 from contextlib import contextmanager
 
@@ -157,6 +158,68 @@ def _handle_auth_error(msg):
         else:
             raise
 
+def _authentication(request_fun):
+    """Decorator to handle autologin and authentication errors.
+
+    *request_fun* is a function taking no arguments that needs to
+    be run with this ``Context`` logged into Splunk.
+
+    ``_authentication``'s behavior depends on whether the
+    ``autologin`` field of ``Context`` is set to ``True`` or
+    ``False``. If it's ``False``, then ``_authentication``
+    aborts if the ``Context`` is not logged in, and raises an
+    ``AuthenticationError`` if an ``HTTPError`` of status 401 is
+    raised in *request_fun*. If it's ``True``, then
+    ``_authentication`` will try at all sensible places to
+    log in before issuing the request.
+
+    If ``autologin`` is ``False``, ``_authentication`` makes
+    one roundtrip to the server if the ``Context`` is logged in,
+    or zero if it is not. If ``autologin`` is ``True``, it's less
+    deterministic, and may make at most three roundtrips (though
+    that would be a truly pathological case).
+
+    :param request_fun: A function of no arguments encapsulating
+                        the request to make to the server.
+
+    **Example**::
+
+        import splunklib.binding as binding
+        c = binding.connect(..., autologin=True)
+        c.logout()
+        def f():
+            c.get("/services")
+            return 42
+        print _authentication(f)
+    """
+    @functools.wraps(request_fun)
+    def wrapper(self, *args, **kwargs):
+        if self.token is None:
+            # Not yet logged in.
+            if self.autologin and self.username and self.password:
+                # This will throw an uncaught
+                # AuthenticationError if it fails.
+                self.login()
+            else:
+                raise AuthenticationError("Request aborted: not logged in.")
+        try:
+            # Issue the request
+            return request_fun(self, *args, **kwargs)
+        except HTTPError as he:
+            if he.status == 401 and self.autologin:
+                # Authentication failed. Try logging in, and then
+                # rerunning the request. If either step fails, throw
+                # an AuthenticationError and give up.
+                with _handle_auth_error("Autologin failed."):
+                    self.login()
+                with _handle_auth_error("Autologin succeeded, but there was an auth error on next request. Something's very wrong."):
+                    return request_fun()
+            elif he.status == 401 and not self.autologin:
+                raise AuthenticationError("Request failed: Session is not logged in.")
+            else:
+                raise
+    return wrapper
+
 def _authority(scheme=DEFAULT_SCHEME, host=DEFAULT_HOST, port=DEFAULT_PORT):
     """Construct a URL authority from the given *scheme*, *host*, and *port*.
 
@@ -353,65 +416,7 @@ def connect(self):
         sock.connect((self.host, self.port))
         return sock
 
-    def _authentication(self, request_fun):
-        """Wrapper to handle autologin and authentication errors.
-
-        *request_fun* is a function taking no arguments that needs to
-        be run with this ``Context`` logged into Splunk.
-
-        ``_authentication``'s behavior depends on whether the
-        ``autologin`` field of ``Context`` is set to ``True`` or
-        ``False``. If it's ``False``, then ``_authentication``
-        aborts if the ``Context`` is not logged in, and raises an
-        ``AuthenticationError`` if an ``HTTPError`` of status 401 is
-        raised in *request_fun*. If it's ``True``, then
-        ``_authentication`` will try at all sensible places to
-        log in before issuing the request.
-
-        If ``autologin`` is ``False``, ``_authentication`` makes
-        one roundtrip to the server if the ``Context`` is logged in,
-        or zero if it is not. If ``autologin`` is ``True``, it's less
-        deterministic, and may make at most three roundtrips (though
-        that would be a truly pathological case).
-
-        :param request_fun: A function of no arguments encapsulating
-                            the request to make to the server.
-
-        **Example**::
-
-            import splunklib.binding as binding
-            c = binding.connect(..., autologin=True)
-            c.logout()
-            def f():
-                c.get("/services")
-                return 42
-            print _authentication(f)
-        """
-        if self.token is None:
-            # Not yet logged in.
-            if self.autologin:
-                # This will throw an uncaught
-                # AuthenticationError if it fails.
-                self.login() 
-            else:
-                raise AuthenticationError("Request aborted: not logged in.")
-        try:
-            # Issue the request
-            return request_fun()
-        except HTTPError as he:
-            if he.status == 401 and self.autologin:
-                # Authentication failed. Try logging in, and then
-                # rerunning the request. If either step fails, throw
-                # an AuthenticationError and give up.
-                with _handle_auth_error("Autologin failed."):
-                    self.login()
-                with _handle_auth_error("Autologin succeeded, but there was an auth error on next request. Something's very wrong."):
-                    return request_fun()
-            elif he.status == 401 and not self.autologin:
-                raise AuthenticationError("Request failed: Session is not logged in.")
-            else:
-                raise
-
+    @_authentication
     def delete(self, path_segment, owner=None, app=None, sharing=None, **query):
         """DELETE at *path_segment* with the given namespace and query.
 
@@ -454,12 +459,11 @@ def delete(self, path_segment, owner=None, app=None, sharing=None, **query):
             c.logout()
             c.delete('apps/local') # raises AuthenticationError
         """
-        def f():
-            path = self.authority + self._abspath(path_segment, owner=owner,
-                                                  app=app, sharing=sharing)
-            return self.http.delete(path, self._auth_headers, **query)
-        return self._authentication(f)
+        path = self.authority + self._abspath(path_segment, owner=owner,
+                                              app=app, sharing=sharing)
+        return self.http.delete(path, self._auth_headers, **query)
 
+    @_authentication
     def get(self, path_segment, owner=None, app=None, sharing=None, **query):
         """GET from *path_segment* with the given namespace and query.
 
@@ -502,12 +506,11 @@ def get(self, path_segment, owner=None, app=None, sharing=None, **query):
             c.logout()
             c.get('apps/local') # raises AuthenticationError
         """
-        def f():
-            path = self.authority + self._abspath(path_segment, owner=owner,
-                                                  app=app, sharing=sharing)
-            return self.http.get(path, self._auth_headers, **query)
-        return self._authentication(f)
+        path = self.authority + self._abspath(path_segment, owner=owner,
+                                              app=app, sharing=sharing)
+        return self.http.get(path, self._auth_headers, **query)
 
+    @_authentication
     def post(self, path_segment, owner=None, app=None, sharing=None, **query):
         """POST to *path_segment* with the given namespace and query.
 
@@ -553,12 +556,11 @@ def post(self, path_segment, owner=None, app=None, sharing=None, **query):
             c.post('saved/searches', name='boris', 
                    search='search * earliest=-1m | head 1')
         """
-        def f():
-            path = self.authority + self._abspath(path_segment, owner=owner, 
-                                                  app=app, sharing=sharing)
-            return self.http.post(path, self._auth_headers, **query)
-        return self._authentication(f)
+        path = self.authority + self._abspath(path_segment, owner=owner, 
+                                              app=app, sharing=sharing)
+        return self.http.post(path, self._auth_headers, **query)
 
+    @_authentication
     def request(self, path_segment, method="GET", headers=[], body="",
                 owner=None, app=None, sharing=None):
         """Issue an arbitrary HTTP request to *path_segment*.
@@ -605,24 +607,24 @@ def request(self, path_segment, method="GET", headers=[], body="",
             c.logout()
             c.get('apps/local') # raises AuthenticationError
         """
-        def f():
-            path = self.authority \
-                + self._abspath(path_segment, owner=owner, 
-                                app=app, sharing=sharing)
-            # all_headers can't be named headers, due to a error in
-            # Python's implementation of closures. In particular:
-            # def f(x):
-            #     def g():
-            #         x = x + "a"
-            #         return x
-            #     return g()
-            # throws UnboundLocalError, claiming that x is not bound.
-            all_headers = headers + self._auth_headers
-            return self.http.request(path,
-                                     {'method': method,
-                                      'headers': all_headers,
-                                      'body': body})
-        return self._authentication(f)
+        path = self.authority \
+            + self._abspath(path_segment, owner=owner, 
+                            app=app, sharing=sharing)
+        # all_headers can't be named headers, due to how
+        # Python implements closures. In particular:
+        # def f(x):
+        #     def g():
+        #         x = x + "a"
+        #         return x
+        #     return g()
+        # throws UnboundLocalError, since x must be either a member of
+        # f's local namespace or g's, and cannot switch between them
+        # during the run of the function.
+        all_headers = headers + self._auth_headers
+        return self.http.request(path,
+                                 {'method': method,
+                                  'headers': all_headers,
+                                  'body': body})
 
     def login(self):
         """Log into the Splunk instance referred to by this ``Context``.