Skip to content

Performance improvement for updating or deleting input #247

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
merged 5 commits into from
Feb 25, 2019
Merged

Performance improvement for updating or deleting input #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7879767
add service.Input function
Feb 23, 2019
a419aa9
fix import issue
Feb 23, 2019
e1bf266
test
Feb 23, 2019
ee7c70a
test
Feb 23, 2019
f6930df
update arg name
Feb 23, 2019
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
7 changes: 7 additions & 0 deletions splunklib/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,13 @@ def info(self):
response = self.get("/services/server/info")
return _filter_content(_load_atom(response, MATCH_ENTRY_CONTENT))

def input(self, path, kind=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big deal, but should we follow Go style practices of adding a new prefix to the declaration?

Like this
def new_input(self, path, kind=None):.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not if we want consistency, so I'm going to say let's not do that here

"""Retrieves an input by path, and optionally kind.

:return: A :class:`Input` object.
"""
return Input(self, path, kind=kind).refresh()
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking Personal Opinion: I like to see these operations broken down atomically because it makes things easier to debug.

new_input = Input(self, path, kind=kind)
new_input.refresh()
return new_input

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fair, I'm just following the style I used for job()


@property
def inputs(self):
"""Returns the collection of inputs configured on this Splunk instance.
Expand Down
16 changes: 12 additions & 4 deletions tests/test_input.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
from __future__ import absolute_import
from __future__ import print_function

import unittest2

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch

from splunklib.binding import HTTPError

from tests import testlib
Expand All @@ -30,6 +28,7 @@

import splunklib.client as client


def highest_port(service, base_port, *kinds):
"""Find the first port >= base_port not in use by any input in kinds."""
highest_port = base_port
Expand All @@ -38,6 +37,7 @@ def highest_port(service, base_port, *kinds):
highest_port = max(port, highest_port)
return highest_port


class TestTcpInputNameHandling(testlib.SDKTestCase):
def setUp(self):
super(TestTcpInputNameHandling, self).setUp()
Expand Down Expand Up @@ -116,6 +116,7 @@ def test_update_restrictToHost_fails(self):
lambda: boris.update(restrictToHost='hilda')
)


class TestRead(testlib.SDKTestCase):
def test_read(self):
inputs = self.service.inputs
Expand Down Expand Up @@ -188,6 +189,7 @@ def test_oneshot_on_nonexistant_file(self):
self.assertRaises(HTTPError,
self.service.inputs.oneshot, name)


class TestInput(testlib.SDKTestCase):
def setUp(self):
super(TestInput, self).setUp()
Expand Down Expand Up @@ -243,7 +245,6 @@ def test_lists_modular_inputs(self):
input = inputs['abcd', 'test2']
self.assertEqual(input.field1, 'boris')


def test_create(self):
inputs = self.service.inputs
for entity in six.itervalues(self._test_entities):
Expand All @@ -264,6 +265,13 @@ def test_read(self):
self.assertEqual(this_entity.name, read_entity.name)
self.assertEqual(this_entity.host, read_entity.host)

def test_read_indiviually(self):
tcp_input = self.service.input(self._test_entities['tcp'].path,
self._test_entities['tcp'].kind)
self.assertIsNotNone(tcp_input)
self.assertTrue('tcp', tcp_input.kind)
self.assertTrue(self._test_entities['tcp'].name, tcp_input.name)

def test_update(self):
inputs = self.service.inputs
for entity in six.itervalues(self._test_entities):
Expand All @@ -273,7 +281,7 @@ def test_update(self):
entity.refresh()
self.assertEqual(entity.host, kwargs['host'])

@unittest2.skip('flaky')
@unittest.skip('flaky')
def test_delete(self):
inputs = self.service.inputs
remaining = len(self._test_entities)-1
Expand Down