Skip to content

Fixed things found in session 1 of the grand Python review. #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
madhadron merged 1 commit into from
Nov 30, 2012
Merged

Fixed things found in session 1 of the grand Python review. #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## 1.0

* Confs has been renamed to Configurations.
* Stanza.submit now takes a dictionary of key/value pairs specifying the stanza instead of a raw string.
* Namespace handling has changed subtly. Code that depends on namespace handling in detail may break.
* Added User.role_entities to return a list of the actual entity objects for the
Expand Down
62 changes: 29 additions & 33 deletions splunklib/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,6 @@ class AmbiguousReferenceException(ValueError):
"""Thrown when the name used to fetch an entity matches more than one entity."""
pass

class EntityDeletedException(Exception):
"""Thrown when the entity that has been referred to has been deleted."""
pass

class InvalidNameException(Exception):
"""Thrown when the specified name contains characters that are not allowed
in Splunk entity names."""
Expand Down Expand Up @@ -209,8 +205,8 @@ def _load_atom_entries(response):
entries = r.feed.get('entry', None)
if entries is None: return None
return entries if isinstance(entries, list) else [entries]
# This rigamarole is because the jobs endpoint doesn't
# returned an entry inside a feed, it just returns and entry.
# The jobs endpoint doesn't returns a bare <entry> element
Copy link
Copy Markdown
Contributor

@itay itay Nov 28, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not English. :) Should be something like:

"The jobs endpoint only returns an outermost element, instead of an outermost element with an element embedded inside."

Copy link
Copy Markdown
Author

@madhadron madhadron Nov 30, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

# instead of an <entry> element inside a <feed> element.
else:
entries = r.get('entry', None)
if entries is None: return None
Expand Down Expand Up @@ -549,9 +545,8 @@ def roles(self):
return Roles(self)

def search(self, query, **kwargs):
"""Runs a oneshot synchronous search using a search query and any
optional arguments you provide. A oneshot search doesn't create a search
job and ID, but rather it returns the search results once completed.
"""Runs a search using a search query and any optional arguments you
provide, and returns a `Job` object representing the search.

:param query: A search query.
:type query: ``string``
Expand All @@ -576,6 +571,8 @@ def search(self, query, **kwargs):
search.

:type kwargs: ``dict``
:rtype: class:`Job`
:returns: An object representing the created job.
"""
return self.jobs.create(query, **kwargs)

Expand Down Expand Up @@ -882,17 +879,14 @@ def __getitem__(self, key):
def _load_atom_entry(self, response):
elem = _load_atom(response, XNAME_ENTRY)
if isinstance(elem, list):
return [x.entry for x in elem]
raise AmbiguousReferenceException("Fetch from server returned multiple entries for name %s." % self.name)
else:
return elem.entry

# Load the entity state record from the given response
def _load_state(self, response):
entry = self._load_atom_entry(response)
if isinstance(entry, list):
raise AmbiguousReferenceException("Fetch from server returned multiple entries for name %s." % self.name)
else:
return _parse_atom_entry(entry)
return _parse_atom_entry(entry)

def _run_method(self, path_segment, **kwargs):
"""Run a method and return the content Record from the returned XML.
Expand All @@ -911,24 +905,26 @@ def _run_method(self, path_segment, **kwargs):
def _proper_namespace(self, owner=None, app=None, sharing=None):
"""Produce a namespace sans wildcards for use in entity requests.

This method handles the case of two entities with the same name in different namespaces
showing up due to wildcards in the service's namespace. We replace the wildcards with the
namespace of the entity we want.
This method tries to fill in the fields of the namespace which are `None`
or wildcard (`'-'`) from the entity's namespace. If that fails, it uses
the service's namespace.

:param owner:
:param app:
:param sharing:
:return:
"""
if owner is None and app is None and sharing is None and\
(self.service.namespace.owner == '-' or self.service.namespace.app == '-'):
# If no namespace is specified and there are wildcards in the service's namespace,
# we need to use the entity's namespace to avoid name collisions.
if 'access' in self._state:
owner = self._state.access.owner
app = self._state.access.app
sharing = self._state.access.sharing
return (owner, app, sharing)
if owner is None and app is None and sharing is None: # No namespace provided
Copy link
Copy Markdown
Contributor

@itay itay Nov 28, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the logic here what we want? Specifically, do we want it so that we use entity.owner if owner is null, not only if owner AND app AND sharing are null? Is what I'm saying making sense?

Copy link
Copy Markdown
Author

@madhadron madhadron Nov 30, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have a global or application namespace instead of a user namespace, some of those the fields will be null, but you don't want to fill in the values from the service in that case.

if self._state is not None and 'access' in self._state:
return (self._state.access.owner,
self._state.access.app,
self._state.access.sharing)
else:
return (self.service.namespace['owner'],
self.service.namespace['app'],
self.service.namespace['sharing'])
else:
return (owner,app,sharing)

def delete(self):
owner, app, sharing = self._proper_namespace()
Expand Down Expand Up @@ -966,13 +962,7 @@ def refresh(self, state=None):
if state is not None:
self._state = state
else:
try:
raw_state = self.read()
except HTTPError, he:
if he.status == 404:
raise EntityDeletedException("Entity %s was already deleted" % self.name)
raw_state['links'] = dict([(k, urllib.unquote(v)) for k,v in raw_state['links'].iteritems()])
self._state = raw_state
self._state = self.read()
return self

@property
Expand Down Expand Up @@ -1043,6 +1033,12 @@ def read(self):
"""Reads the current state of the entity from the server."""
response = self.get()
results = self._load_state(response)
# In lower layers of the SDK, we end up trying to URL encode
# text to be dispatched via HTTP. However, these links are already
# URL encoded when they arrive, and we need to mark them as such.
unquoted_links = dict([(k, UrlEncoded(v, skip_encode=True))
for k,v in results['links'].iteritems()])
results['links'] = unquoted_links
return results

def reload(self):
Expand Down
2 changes: 1 addition & 1 deletion tests/test_input.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ def test_delete(self):
inputs.delete, name)
self.service.inputs.delete(kind, name)
self.assertFalse((kind,name) in inputs)
self.assertRaises(client.EntityDeletedException,
self.assertRaises(client.HTTPError,
input_entity.refresh)
remaining -= 1

Expand Down
2 changes: 1 addition & 1 deletion tests/test_role.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def test_delete(self):
self.assertTrue(self.role_name in self.service.roles)
self.service.roles.delete(self.role_name)
self.assertFalse(self.role_name in self.service.roles)
self.assertRaises(client.EntityDeletedException, self.role.refresh)
self.assertRaises(client.HTTPError, self.role.refresh)

def test_grant_and_revoke(self):
self.assertFalse('edit_user' in self.role.capabilities)
Expand Down
2 changes: 1 addition & 1 deletion tests/test_saved_search.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def test_delete(self):
self.assertTrue(self.saved_search_name in self.service.saved_searches)
self.service.saved_searches.delete(self.saved_search_name)
self.assertFalse(self.saved_search_name in self.service.saved_searches)
self.assertRaises(client.EntityDeletedException,
self.assertRaises(client.HTTPError,
self.saved_search.refresh)


Expand Down
2 changes: 1 addition & 1 deletion tests/test_user.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def test_create(self):
def test_delete(self):
self.service.users.delete(self.username)
self.assertFalse(self.username in self.service.users)
with self.assertRaises(client.EntityDeletedException):
with self.assertRaises(client.HTTPError):
self.user.refresh()

def test_update(self):
Expand Down