So, how do we stand on this ? Thanks, Jose
On Fri, Aug 16, 2013 at 09:29:46AM +0200, Thomas Thrainer wrote: > Oh, and BTW, there is a check for VALUE_HS_NOTHING in > utils.__init__.py:199. We might have to look at this as well. > > > On Fri, Aug 16, 2013 at 9:27 AM, Thomas Thrainer <[email protected]>wrote: > > > Hi, > > > > It looks as if QA is broken with this patch. Did you try to run it on your > > test cluster? I assume the problem has something do to with the > > configuration being parsed in Python and Haskell as well. > > I assumed (based on git blame) that you introduced the VALUE_HS_NOTHING > > constant, but if Michele knows more about this, feel free to assign the bug > > to him and then lets see next week why this was done. > > > > Cheers, > > Thomas > > > > > > On Wed, Aug 14, 2013 at 9:34 PM, Jose A. Lopes <[email protected]>wrote: > > > >> From: "Jose A. Lopes" <[email protected]> > >> > >> Fix 'NICC_DEFAULTS' to use 'None' instead of 'VALUE_HS_NOTHING' since > >> that is the value expected by the opcodes, and add 2 tests to ensure > >> 'NICC_DEFAULTS' is a valid default value for the 'nicparams' parameter > >> of 'ClusterSetParams' opcode. > >> > >> Signed-off-by: Jose A. Lopes <[email protected]> > >> --- > >> > >> This patch SHOULD fix issue 552, but I couldn't run QA because master > >> is broken (see my previous email). Maybe I need to install something > >> and if so please let me know and I will run QA and resubmit the patch. > >> > >> In any case, this patch replace the use of 'VALUE_HS_NOTHING' with > >> 'None'. It seems that Michele created 'VALUE_HS_NOTHING' to represent > >> the value 'Nothing' from Haskell. However, opcodes use 'None' to > >> represent that value. Therefore, we should ask Michele if > >> 'VALUE_HS_NOTHING' is strictly necessary for some other reason, or > >> otherwise it can be completely replaced by 'None'. > >> > >> lib/constants.py | 2 +- > >> test/py/cmdlib/cluster_unittest.py | 5 +++++ > >> test/py/ganeti.opcodes_unittest.py | 5 +++++ > >> 3 files changed, 11 insertions(+), 1 deletion(-) > >> > >> diff --git a/lib/constants.py b/lib/constants.py > >> index 84b5fac..6e81c7e 100644 > >> --- a/lib/constants.py > >> +++ b/lib/constants.py > >> @@ -2286,7 +2286,7 @@ del _LV_DEFAULTS, _DRBD_DEFAULTS > >> NICC_DEFAULTS = { > >> NIC_MODE: NIC_MODE_BRIDGED, > >> NIC_LINK: DEFAULT_BRIDGE, > >> - NIC_VLAN: VALUE_HS_NOTHING, > >> + NIC_VLAN: None, > >> } > >> > >> # All of the following values are quite arbitrarily - there are no > >> diff --git a/test/py/cmdlib/cluster_unittest.py > >> b/test/py/cmdlib/cluster_unittest.py > >> index 55676c6..6809384 100644 > >> --- a/test/py/cmdlib/cluster_unittest.py > >> +++ b/test/py/cmdlib/cluster_unittest.py > >> @@ -611,6 +611,11 @@ class TestLUClusterSetParams(CmdlibTestCase): > >> op = opcodes.OpClusterSetParams(nicparams=nicparams) > >> self.ExecOpCode(op) > >> > >> + def testNicparamsNiccDefaults(self): > >> + nicparams = objects.FillDict(constants.NICC_DEFAULTS, {}) > >> + op = opcodes.OpClusterSetParams(nicparams=nicparams) > >> + self.ExecOpCode(op) > >> + > >> def testDefaultHvparams(self): > >> hvparams = constants.HVC_DEFAULTS > >> op = opcodes.OpClusterSetParams(hvparams=hvparams) > >> diff --git a/test/py/ganeti.opcodes_unittest.py b/test/py/ > >> ganeti.opcodes_unittest.py > >> index 63658c3..fb65466 100755 > >> --- a/test/py/ganeti.opcodes_unittest.py > >> +++ b/test/py/ganeti.opcodes_unittest.py > >> @@ -404,5 +404,10 @@ class TestOpInstanceSetParams(unittest.TestCase): > >> self.assertTrue(fn([[constants.DDM_ADD, {param: param}]])) > >> > >> > >> +class TestOpClusterSetParams(unittest.TestCase): > >> + def testNicparamsDefaults(self): > >> + self.assertTrue(ht.TINicParams(constants.NICC_DEFAULTS)) > >> + > >> + > >> if __name__ == "__main__": > >> testutils.GanetiTestProgram() > >> -- > >> 1.8.3 > >> > >> > > > > > > -- > > Thomas Thrainer | Software Engineer | [email protected] | > > > > Google Germany GmbH > > Dienerstr. 12 > > 80331 München > > > > Registergericht und -nummer: Hamburg, HRB 86891 > > Sitz der Gesellschaft: Hamburg > > Geschäftsführer: Graham Law, Christine Elizabeth Flores > > > > > > -- > Thomas Thrainer | Software Engineer | [email protected] | > > Google Germany GmbH > Dienerstr. 12 > 80331 München > > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg > Geschäftsführer: Graham Law, Christine Elizabeth Flores
