Giuseppe Lavagetto has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/405301 )
Change subject: cli.tool: drop the "find" interface
......................................................................
cli.tool: drop the "find" interface
Since its function is fully covered by 'select', which ends up being
both more powerful and equally efficient, let's drop it.
Change-Id: I62633d7e180efd34d7f468c4b71e3ed7220a0f35
---
M conftool/cli/tool.py
M conftool/tests/integration/test_tool.py
M conftool/tests/unit/test_cli_tool.py
M setup.py
4 files changed, 54 insertions(+), 108 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/software/conftool
refs/changes/01/405301/1
diff --git a/conftool/cli/tool.py b/conftool/cli/tool.py
index 18f5e01..32abf34 100644
--- a/conftool/cli/tool.py
+++ b/conftool/cli/tool.py
@@ -27,13 +27,22 @@
pass
-class ToolCli(object):
+class ToolCliBase(object):
def __init__(self, args):
self.args = args
self._load_schema()
- self._tags = self.args.taglist.split(',')
self.irc = logging.getLogger('conftool.announce')
+
+ def tags(self):
+ return []
+
+ def announce(self):
+ if self._action != 'get' and not self.args.quiet:
+ self.irc.warning(
+ "conftool action : %s; selector: %s", self._action,
+ self._namedef
+ )
def _load_schema(self):
self._schema = loader.Schema.from_file(self.args.schema)
@@ -50,6 +59,42 @@
c = configuration.get(self.args.config)
KVObject.setup(c)
setup_irc(c)
+
+ def _run_action(self):
+ fail = False
+ for obj in self.host_list():
+ try:
+ a = action.Action(obj, self._action)
+ msg = a.run()
+ except action.ActionError as e:
+ fail = True
+ _log.error("Invalid action, reason: %s", str(e))
+ except BackendError as e:
+ fail = True
+ _log.error("Error when trying to %s on %s", self._action,
+ self._namedef)
+ _log.error("Failure writing to the kvstore: %s", str(e))
+ except Exception as e:
+ fail = True
+ _log.error("Error when trying to %s on %s", self._action,
+ self._namedef)
+ _log.exception("Generic action failure: %s", str(e))
+ else:
+ if sys.version_info[0] == 2: # Python 2
+ msg = msg.decode('utf-8')
+ print(msg)
+ if not fail:
+ self.announce()
+ return True
+ else:
+ return False
+
+
+class ToolCli(ToolCliBase):
+
+ def __init__(self, args):
+ super(ToolCli, self).__init__(args)
+ self._tags = self.args.taglist.split(',')
@property
def tags(self):
@@ -112,35 +157,6 @@
self._action, self._namedef = unit
return self._run_action()
- def _run_action(self):
- fail = False
- for obj in self.host_list():
- try:
- a = action.Action(obj, self._action)
- msg = a.run()
- except action.ActionError as e:
- fail = True
- _log.error("Invalid action, reason: %s", str(e))
- except BackendError as e:
- fail = True
- _log.error("Error when trying to %s on %s", self._action,
- self._namedef)
- _log.error("Failure writing to the kvstore: %s", str(e))
- except Exception as e:
- fail = True
- _log.error("Error when trying to %s on %s", self._action,
- self._namedef)
- _log.exception("Generic action failure: %s", str(e))
- else:
- if sys.version_info[0] == 2: # Python 2
- msg = msg.decode('utf-8')
- print(msg)
- if not fail:
- self.announce()
- return True
- else:
- return False
-
@staticmethod
def raise_warning():
if not sys.stdin.isatty() or not sys.stdout.isatty():
@@ -158,30 +174,7 @@
sys.exit(1)
-class ToolCliFind(ToolCli):
- """Subclass used for the --find mode"""
- def __init__(self, args):
- self.args = args
- self._load_schema()
- self.irc = logging.getLogger('conftool.announce')
-
- @property
- def tags(self):
- return []
-
- def host_list(self):
- for o in self.entity.find(self._namedef):
- yield o
-
- def announce(self):
- if self._action != 'get' and not self.args.quiet:
- self.irc.warning(
- "conftool action : %s; selector: %s", self._action,
- self._namedef
- )
-
-
-class ToolCliByLabel(ToolCliFind):
+class ToolCliByLabel(ToolCliBase):
"""Subclass used for the select mode"""
def __init__(self, args):
super(ToolCliByLabel, self).__init__(args)
@@ -265,14 +258,11 @@
'taglist',
help="List of comma-separated tags; they need to "
"match the base tags of the object type you chose.")
+ tags.add_argument('--action', action="append", metavar="ACTIONS",
+ help="the action to take: "
+ " [set/k1=v1:k2=v2...|get|delete]"
+ " node|all|re:<regex>|find:node", nargs=2)
- find = subparsers.add_parser('find',
- help="Find all instances of the node by name")
- for subparser in [tags, find]:
- subparser.add_argument('--action', action="append", metavar="ACTIONS",
- help="the action to take: "
- " [set/k1=v1:k2=v2...|get|delete]"
- " node|all|re:<regex>|find:node", nargs=2)
select = subparsers.add_parser('select',
help="Select nodes via tag selectors")
select.add_argument(
@@ -290,7 +280,7 @@
"""Basic mangling of the command line arguments"""
# Backwards compatibility. Ugly but passable
for i, arg in enumerate(cmdline):
- if arg in ['--tags', '--find']:
+ if arg in ['--tags']:
cmdline[i] = arg.replace('--', '')
return cmdline
@@ -308,11 +298,8 @@
logging.basicConfig(level=logging.WARN)
try:
-
if args.mode == 'select':
cli = ToolCliByLabel(args)
- elif args.mode == 'find':
- cli = ToolCliFind(args)
else:
cli = ToolCli(args)
except ObjectTypeError:
diff --git a/conftool/tests/integration/test_tool.py
b/conftool/tests/integration/test_tool.py
index 828a95b..b72e921 100644
--- a/conftool/tests/integration/test_tool.py
+++ b/conftool/tests/integration/test_tool.py
@@ -62,19 +62,6 @@
# Check the irc message was not sent for a get operation
self.assertEqual(self.irc.emit.called, False)
- def test_find_node(self):
- args = ['find', '--action', 'get', 'cp1008']
- output = self.output_for(args)
- self.assertEqual(len(output), 3)
- for serv in output:
- self.assertTrue(serv['tags'].startswith('dc=eqiad'))
- # Test that old-style parameters are still valid
- args = ['--find', '--action', 'get', 'cp1008']
- output = self.output_for(args)
- self.assertEqual(len(output), 3)
- # Check the irc message was not sent for a get operation
- self.assertEqual(self.irc.emit.called, False)
-
def test_change_node_regexp(self):
"""
Changing values according to a regexp
diff --git a/conftool/tests/unit/test_cli_tool.py
b/conftool/tests/unit/test_cli_tool.py
index 949ce4b..eecf3da 100644
--- a/conftool/tests/unit/test_cli_tool.py
+++ b/conftool/tests/unit/test_cli_tool.py
@@ -50,34 +50,6 @@
self.assertItemsEqual(t.tags, ['a', 'b', 'apache2'])
else:
self.assertCountEqual(t.tags, ['a', 'b', 'apache2'])
- args = self._mock_args(mode='find')
- t = tool.ToolCliFind(args)
- self.assertEqual(t.tags, [])
- args = self._mock_args(
- mode='find',
- object_type='node',
- )
- t = tool.ToolCliFind(args)
- self.assertEqual(t.entity.__name__, 'Node')
-
- def test_host_list_find(self):
- args = self._mock_args(
- mode="find",
- object_type='node',
- )
-
- t = tool.ToolCliFind(args)
- t._namedef = 'cp1048.example.com'
- res = [
- node.Node('test', 'cache',
- 'ssl', 'cp1048.example.com'),
- node.Node('test', 'cache',
- 'http', 'cp1048.example.com'),
- ]
- t.entity.find = mock.MagicMock(return_value=res)
- tres = [o for o in t.host_list()]
- self.assertEqual(tres, res)
- t.entity.find.assert_called_with(t._namedef)
def test_hosts_list_tags(self):
"""Tests getting the host list"""
diff --git a/setup.py b/setup.py
index 69f4b4f..03e7a75 100755
--- a/setup.py
+++ b/setup.py
@@ -4,7 +4,7 @@
setup(
name='conftool',
- version='0.4.1',
+ version='0.5.0',
description='Tools to interoperate with distributed k/v stores',
author='Joe',
author_email='[email protected]',
--
To view, visit https://gerrit.wikimedia.org/r/405301
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I62633d7e180efd34d7f468c4b71e3ed7220a0f35
Gerrit-PatchSet: 1
Gerrit-Project: operations/software/conftool
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Lavagetto <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
