Giuseppe Lavagetto has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/405303 )
Change subject: Refactor conftool.action
......................................................................
Refactor conftool.action
Split the Action class, which had grown out of proportion, to a separate
list of smaller Action classes.
Change-Id: I6cccb9e62887aa9151ac2ede50e923689cfe57b2
---
M conftool/action.py
M conftool/cli/tool.py
M conftool/tests/unit/test_action.py
3 files changed, 77 insertions(+), 63 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/software/conftool
refs/changes/03/405303/1
diff --git a/conftool/action.py b/conftool/action.py
index 1870435..76d3c84 100644
--- a/conftool/action.py
+++ b/conftool/action.py
@@ -12,30 +12,60 @@
pass
-class Action(object):
+def get_action(obj, act):
+ prefix = act[:3]
+ if prefix == 'get':
+ return GetAction(obj)
+ elif act == 'delete':
+ return DelAction(obj)
+ elif prefix == 'set':
+ return SetAction(obj, act[4:])
+ else:
+ raise ActionError("Could not parse action %s" % act)
+
+
+class GetAction(object):
+ """Action to perform when a get request is involved"""
+ def __init__(self, obj):
+ self.entity = obj
+
+ def run(self):
+ self.entity.fetch()
+ if self.entity.exists:
+ return str(self.entity)
+ else:
+ return "%s not found" % self.entity.name
+
+
+class DelAction(GetAction):
+ """Action to perform when deleting an object"""
+ def run(self):
+ self.entity.delete()
+ entity_type = self.entity.__class__.__name__,
+ return "Deleted %s %s." % (entity_type,
+ self.entity.name)
+
+
+class SetAction(object):
def __init__(self, obj, act):
+ """Action to perform when editing an object"""
self.entity = obj
- self.action, self.args = self._parse_action(act)
+ if not self.entity.exists:
+ raise ActionError("Entity %s doesn't exist" % self.entity.name)
+
+ self.args = self._parse_action(act)
self.description = ""
- def _parse_action(self, act):
- # TODO: Move this to the cli.tool submodule
+ def _parse_action(self, arg):
# TODO: make the parsing of the argument a bit more formal
- if act.startswith('get'):
- return ('get', None)
- elif act.startswith('delete'):
- return ('delete', None)
- elif not act.startswith('set/'):
- raise ActionError("Cannot parse action %s" % act)
- set_arg = act.replace('set/', '', 1)
- if set_arg.startswith('@'):
- return ('set', self._from_file(set_arg))
+ if arg.startswith('@'):
+ return self._from_file(arg)
try:
- values = dict((el.strip().split('=')) for el in set_arg.split(':'))
+ values = dict((el.strip().split('=')) for el in arg.split(':'))
except Exception:
- raise ActionError("Could not parse set instructions: %s" % set_arg)
- return ('set', self._from_cli(values))
+ raise ActionError("Could not parse set instructions: %s" % arg)
+ return self._from_cli(values)
def _from_cli(self, values):
for k, v in values.items():
@@ -70,33 +100,19 @@
return values
def run(self):
- if self.action == 'get':
- self.entity.fetch()
- if self.entity.exists:
- return str(self.entity)
- else:
- return "%s not found" % self.entity.name
- elif self.action == 'delete':
- self.entity.delete()
- entity_type = self.entity.__class__.__name__,
- return "Deleted %s %s." % (entity_type,
- self.entity.name)
- elif self.action == 'set':
- if not self.entity.exists:
- raise ActionError("Entity %s doesn't exist" % self.entity.name)
- # Validate the new data *before* updating the object
- try:
- self.entity.validate(self.args)
- except Exception as e:
- raise ActionValidationError("The provided data is not valid:
%s" % e)
+ # Validate the new data *before* updating the object
+ try:
+ self.entity.validate(self.args)
+ except Exception as e:
+ raise ActionValidationError("The provided data is not valid: %s" %
e)
- desc = []
- for (k, v) in self.args.items():
- curval = getattr(self.entity, k)
- if v != curval:
- msg = "%s: %s changed %s => %s" % (
- self.entity.name, k,
- curval, v)
- desc.append(msg)
- self.entity.update(self.args)
- return "\n".join(desc)
+ desc = []
+ for (k, v) in self.args.items():
+ curval = getattr(self.entity, k)
+ if v != curval:
+ msg = "%s: %s changed %s => %s" % (
+ self.entity.name, k,
+ curval, v)
+ desc.append(msg)
+ self.entity.update(self.args)
+ return "\n".join(desc)
diff --git a/conftool/cli/tool.py b/conftool/cli/tool.py
index 32abf34..056437d 100644
--- a/conftool/cli/tool.py
+++ b/conftool/cli/tool.py
@@ -64,7 +64,7 @@
fail = False
for obj in self.host_list():
try:
- a = action.Action(obj, self._action)
+ a = action.get_action(obj, self._action)
msg = a.run()
except action.ActionError as e:
fail = True
diff --git a/conftool/tests/unit/test_action.py
b/conftool/tests/unit/test_action.py
index 7a68c3a..05ace73 100644
--- a/conftool/tests/unit/test_action.py
+++ b/conftool/tests/unit/test_action.py
@@ -2,7 +2,7 @@
import unittest
from conftool import configuration
-from conftool.action import Action, ActionError
+from conftool.action import get_action, ActionError, GetAction, DelAction,
SetAction
from conftool.kvobject import KVObject
from conftool.tests.unit import MockBackend, MockEntity
from conftool.types import get_validator
@@ -13,41 +13,39 @@
KVObject.backend = MockBackend({})
KVObject.config = configuration.Config(driver="")
self.entity = MockEntity('Foo', 'Bar', 'test')
-
+ self.entity.exists = True
def test_init(self):
"""
Test initialization of the Action object
"""
# Get action
- a = Action(self.entity, 'get')
+ a = get_action(self.entity, 'get')
self.assertEqual(a.entity, self.entity)
- self.assertEqual(a.action, 'get')
- self.assertIsNone(a.args)
+ self.assertEqual(a.__class__, GetAction)
# Delete action
- a = Action(self.entity, 'delete')
- self.assertEqual(a.action, 'delete')
- self.assertIsNone(a.args)
+ a = get_action(self.entity, 'delete')
+ self.assertEqual(a.__class__, DelAction)
# Set from file
- with mock.patch('conftool.action.Action._from_file') as mocker:
+ with mock.patch('conftool.action.SetAction._from_file') as mocker:
values = {'a': 10, 'b': 'test test'}
mocker.return_value = values
- a = Action(self.entity, 'set/@filename.yaml')
- self.assertEqual(a.action, 'set')
+ a = get_action(self.entity, 'set/@filename.yaml')
+ self.assertEqual(a.__class__, SetAction)
self.assertEqual(a.args, values)
# Set from cli
- a = Action(self.entity, 'set/a=10:b=test test')
- self.assertEqual(a.action, 'set')
+ a = get_action(self.entity, 'set/a=10:b=test test')
+ self.assertEqual(a.__class__, SetAction)
self.assertEqual(a.args, {'a': '10', 'b': 'test test'})
- self.assertRaises(ActionError, Action, self.entity, 'set/a=1:')
- a = Action(self.entity, 'set/a=true:b=a,foo,bar')
+ self.assertRaises(ActionError, get_action, self.entity, 'set/a=1:')
+ a = get_action(self.entity, 'set/a=true:b=a,foo,bar')
self.assertEqual(a.args, {'a': 'true', 'b': 'a,foo,bar'})
def test_from_cli(self):
"""
Test parsing of cli-provided arguments
"""
- a = Action(self.entity, 'get')
+ a = get_action(self.entity, 'set/bar=ac')
a.entity._schema['bar'] = get_validator('list')
self.assertEqual(a._from_cli({'bar': 'abc,def,ghi'}),
{'bar': ['abc', 'def', 'ghi']})
@@ -57,7 +55,7 @@
del a.entity._schema['bar']
def test_run(self):
- a = Action(self.entity, 'get')
+ a = get_action(self.entity, 'get')
a.entity.fetch = mock.Mock()
a.entity.exists = False
self.assertEqual(a.run(), "test not found")
--
To view, visit https://gerrit.wikimedia.org/r/405303
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6cccb9e62887aa9151ac2ede50e923689cfe57b2
Gerrit-PatchSet: 1
Gerrit-Project: operations/software/conftool
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Lavagetto <glavage...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
