LGTM Thanks,
Guido On Fri, Feb 22, 2013 at 7:53 AM, Michael Hanselmann <[email protected]> wrote: > - Commit 4a90bd4 contained a rather large number of style violations: > - Indentation/sequence formatting > - Wrapping of long lines > - Docstrings whose first line was wrapped > - A stray backslash in a docstring > - opcodes: Don't check for list or tuple. None of the other opcodes does > it explicitely. As long as the length and the items match the value is > accepted. > - server/noded: “if variable” doesn't test for None, but False > --- > lib/opcodes.py | 10 ++++------ > lib/rapi/rlib2.py | 9 ++++----- > lib/server/noded.py | 12 +++++++----- > test/py/ganeti.rapi.rlib2_unittest.py | 6 ++---- > 4 files changed, 17 insertions(+), 20 deletions(-) > > diff --git a/lib/opcodes.py b/lib/opcodes.py > index 43f4973..59af3f6 100644 > --- a/lib/opcodes.py > +++ b/lib/opcodes.py > @@ -1452,13 +1452,11 @@ class OpInstanceReboot(OpCode): > ("reboot_type", ht.NoDefault, ht.TElemOf(constants.REBOOT_TYPES), > "How to reboot instance"), > ("reason", (constants.INSTANCE_REASON_SOURCE_UNKNOWN, None), > - ht.TAnd(ht.TOr(ht.TList, ht.TTuple), > - ht.TIsLength(2), > + ht.TAnd(ht.TIsLength(2), > ht.TItems([ > - ht.TElemOf(constants.INSTANCE_REASON_SOURCES), > - ht.TMaybeString, > - ]) > - ), > + ht.TElemOf(constants.INSTANCE_REASON_SOURCES), > + ht.TMaybeString, > + ])), > "The reason why the reboot is happening"), > ] > OP_RESULT = ht.TNone > diff --git a/lib/rapi/rlib2.py b/lib/rapi/rlib2.py > index 89a99c5..a475151 100644 > --- a/lib/rapi/rlib2.py > +++ b/lib/rapi/rlib2.py > @@ -1040,11 +1040,10 @@ class > R_2_instances_name_reboot(baserlib.OpcodeResource): > self.queryargs.get("type", [constants.INSTANCE_REBOOT_HARD])[0], > "ignore_secondaries": > bool(self._checkIntVariable("ignore_secondaries")), > "dry_run": self.dryRun(), > - "reason": ( > - constants.INSTANCE_REASON_SOURCE_RAPI, > - self._checkStringVariable("reason_text", > - default=constants.INSTANCE_REASON_REBOOT), > - ) > + "reason": > + (constants.INSTANCE_REASON_SOURCE_RAPI, > + self._checkStringVariable("reason_text", > + > default=constants.INSTANCE_REASON_REBOOT)), > }) > > > diff --git a/lib/server/noded.py b/lib/server/noded.py > index aa9243f..96ad1ac 100644 > --- a/lib/server/noded.py > +++ b/lib/server/noded.py > @@ -113,12 +113,14 @@ def _DecodeImportExportIO(ieio, ieioargs): > > > def _DefaultAlternative(value, default): > - """Returns the given value, unless it is None. In that case, returns a > - default alternative. > + """Returns value or, if evaluating to False, a default value. > > - @param value: The value to return if it is not None. > - @param default: The value to return as a default alternative. > - @return: The given value or the default alternative.\ > + Returns the given value, unless it evaluates to False. In the latter case > the > + default value is returned. > + > + @param value: Value to return if it doesn't evaluate to False > + @param default: Default value > + @return: Given value or the default > > """ > if value: > diff --git a/test/py/ganeti.rapi.rlib2_unittest.py > b/test/py/ganeti.rapi.rlib2_unittest.py > index 77c4435..e19e800 100755 > --- a/test/py/ganeti.rapi.rlib2_unittest.py > +++ b/test/py/ganeti.rapi.rlib2_unittest.py > @@ -370,7 +370,7 @@ class TestInstanceReboot(unittest.TestCase): > handler = _CreateHandler(rlib2.R_2_instances_name_reboot, ["inst847"], { > "dry-run": ["1"], > "ignore_secondaries": ["1"], > - "reason_text": ["System update"] > + "reason_text": ["System update"], > }, {}, clfactory) > job_id = handler.POST() > > @@ -385,9 +385,7 @@ class TestInstanceReboot(unittest.TestCase): > self.assertTrue(op.ignore_secondaries) > self.assertTrue(op.dry_run) > self.assertEqual(op.reason, > - (constants.INSTANCE_REASON_SOURCE_RAPI, > - "System update", > - )) > + (constants.INSTANCE_REASON_SOURCE_RAPI, "System > update")) > > self.assertRaises(IndexError, cl.GetNextSubmittedJob) > > -- > 1.8.1.3 > -- Guido Trotter Ganeti engineering Google Germany
