On Fri, Aug 30, 2013 at 12:29 PM, Jose A. Lopes <[email protected]>wrote:
> So, how do we stand on this ? > > I think it's related to the other mail that I sent a few days ago, about the JSON encoding of Maybe being different in Haskell and Python. I'm afraid the python JSON loader has to be modified to correctly turn Nothings in Nones, and Justs into actual values. > 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 > -- 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
