Raise error when serialization type is unkown

See merge request !436

slapos/cli/request.py

@@ -40,7 +40,8 @@ from slapos.cli.config import ClientConfigCommand
 from slapos.client import (ClientConfig, _getSoftwareReleaseFromSoftwareString,
                            init)
 from slapos.slap import ResourceNotReady
-from slapos.util import SoftwareReleaseSchema, SoftwareReleaseSerialisation
+from slapos.util import (SoftwareReleaseSchema, SoftwareReleaseSerialisation,
+                         UndefinedSerializationError)
 
 try:
     from typing import IO, Dict
@@ -158,10 +159,10 @@ def do_request(logger, conf, local):
         conf.software_url = local[conf.software_url]
 
     software_schema = SoftwareReleaseSchema(conf.software_url, conf.type)
-    software_schema_serialisation = software_schema.getSerialisation()
     parameters = conf.parameters
     if conf.parameters_file:
-        parameters = getParametersFromFile(conf.parameters_file, software_schema_serialisation)
+        # getSerialisation must throw an exception if serialization cannot be found
+        parameters = getParametersFromFile(conf.parameters_file, software_schema.getSerialisation())
     try:
         partition = local['slap'].registerOpenOrder().request(
             software_release=conf.software_url,
@@ -175,9 +176,12 @@ def do_request(logger, conf, local):
         logger.info('Instance requested.\nState is : %s.', partition.getState())
         logger.info('Connection parameters of instance are:')
         connection_parameter_dict = partition.getConnectionParameterDict()
-        if software_schema_serialisation == SoftwareReleaseSerialisation.JsonInXml:
-            if '_' in connection_parameter_dict:
-                connection_parameter_dict = json.loads(connection_parameter_dict['_'])
+        try:
+            if software_schema.getSerialisation() == SoftwareReleaseSerialisation.JsonInXml:
+                if '_' in connection_parameter_dict:
+                    connection_parameter_dict = json.loads(connection_parameter_dict['_'])
+        except UndefinedSerializationError:
+            pass
         logger.info(StrPrettyPrinter().pformat(connection_parameter_dict))
         logger.info('You can rerun the command to get up-to-date information.')
     except ResourceNotReady:

slapos/slap/slap.py

@@ -49,8 +49,9 @@ import six
 from .exception import ResourceNotReady, ServerError, NotFoundError, \
           ConnectionError
 from .hateoas import SlapHateoasNavigator, ConnectionHelper
-from slapos.util import (SoftwareReleaseSchema, bytes2str, calculate_dict_hash,
-                         dict2xml, dumps, loads, unicode2str, xml2dict)
+from slapos.util import (SoftwareReleaseSchema, UndefinedSerializationError,
+                         bytes2str, calculate_dict_hash, dict2xml, dumps, loads,
+                         unicode2str, xml2dict)
 
 from xml.sax import saxutils
 from zope.interface import implementer
@@ -100,6 +101,11 @@ class SlapRequester(SlapDocument):
         "Request parameters do not validate against schema definition:\n{e}".format(e=e),
         UserWarning,
       )
+    except UndefinedSerializationError as e:
+      warnings.warn(
+        "No serialization type found:\n{e}".format(e=e),
+        UserWarning,
+      )
     except Exception as e:
       # note that we intentionally catch wide exceptions, so that if anything
       # is wrong with fetching the schema or the schema itself this does not

slapos/tests/test_cli.py

@@ -40,7 +40,7 @@ import pkg_resources
 from contextlib import contextmanager
 from mock import patch, create_autospec
 import mock
-from slapos.util import sqlite_connect, bytes2str
+from slapos.util import sqlite_connect, bytes2str, UndefinedSerializationError
 from slapos.slap.slap import DEFAULT_SOFTWARE_TYPE
 
 import slapos.cli.console
@@ -906,23 +906,22 @@ class TestCliRequest(CliMixin):
     ])
 
 
-class TestCliRequestParametersFileJson(CliMixin):
+class TestCliRequestParameterFile(CliMixin):
   """Request with --parameter-file, with a .json file.
   """
   expected_partition_parameter_kw = {'foo': ['bar']}
 
   def _makeParameterFile(self):
-    f = tempfile.NamedTemporaryFile(suffix='.json', mode='w', delete=False)
+    f = tempfile.NamedTemporaryFile(
+        suffix=self.parameter_file_suffix,
+        mode='w', delete=False,
+    )
     self.addCleanup(os.unlink, f.name)
-    f.write(textwrap.dedent('''\
-    {
-      "foo": ["bar"]
-    }
-    '''))
+    f.write(textwrap.dedent(self.parameter_file_content))
     f.flush()
     return f.name
 
-  def test_request_parameters_file(self):
+  def _request_parameters_file_setup(self):
     self.conf.reference = 'instance reference'
     self.conf.software_url = 'software URL'
     self.conf.parameters =  None
@@ -934,28 +933,66 @@ class TestCliRequestParametersFileJson(CliMixin):
     self.conf.state = None
     self.conf.slave = False
 
-    with patch.object(
-        slapos.slap.slap,
-        'registerOpenOrder',
-        return_value=mock.create_autospec(slapos.slap.OpenOrder)) as registerOpenOrder:
-      slapos.cli.request.do_request(self.logger, self.conf, self.local)
 
-    registerOpenOrder().request.assert_called_once_with(
-        software_release='software URL',
-        partition_reference='instance reference',
-        partition_parameter_kw=self.expected_partition_parameter_kw,
-        software_type=None,
-        filter_kw={'computer_guid': 'COMP-1234'},
-        state=None,
-        shared=False,
-    )
-    self.logger.info.assert_any_call(
-        'Requesting %s as instance of %s...',
-        'instance reference',
-        'software URL',
+class TestCliRequestParameterFileUndefinedSerialization(TestCliRequestParameterFile):
+  """Request with --parameter-file, without defining serialization type.
+  """
+  parameter_file_suffix = ''
+  parameter_file_content = '''\
+  {
+    "foo": ["bar"]
+  }
+  '''
+
+  def test_request_parameters_file(self):
+    self._request_parameters_file_setup()
+    self.assertRaises(
+        UndefinedSerializationError,
+        slapos.cli.request.do_request,
+        self.logger,
+        self.conf,
+        self.local,
     )
 
 
+class TestCliRequestParametersFileJson(TestCliRequestParameterFile):
+  """Request with --parameter-file, with a .json file.
+  """
+  parameter_file_suffix = '.json'
+  parameter_file_content = '''\
+  {
+    "foo": ["bar"]
+  }
+  '''
+  serialization = 'xml'
+
+  def test_request_parameters_file(self):
+    self._request_parameters_file_setup()
+    with mock.patch(
+        'slapos.cli.request.SoftwareReleaseSchema.getSerialisation',
+        return_value=self.serialization):
+      with patch.object(
+          slapos.slap.slap,
+          'registerOpenOrder',
+          return_value=mock.create_autospec(slapos.slap.OpenOrder)) as registerOpenOrder:
+        slapos.cli.request.do_request(self.logger, self.conf, self.local)
+
+      registerOpenOrder().request.assert_called_once_with(
+          software_release='software URL',
+          partition_reference='instance reference',
+          partition_parameter_kw=self.expected_partition_parameter_kw,
+          software_type=None,
+          filter_kw={'computer_guid': 'COMP-1234'},
+          state=None,
+          shared=False,
+      )
+      self.logger.info.assert_any_call(
+          'Requesting %s as instance of %s...',
+          'instance reference',
+          'software URL',
+      )
+
+
 class TestCliRequestParametersFileJsonJsonInXMLSerialisation(
     TestCliRequestParametersFileJson):
   """Request with --parameter-file, with a .json file and a software using
@@ -963,68 +1000,40 @@ class TestCliRequestParametersFileJsonJsonInXMLSerialisation(
   serialised with {'_': json.dumps(params)}
   """
   expected_partition_parameter_kw = {"_": "{\"foo\": [\"bar\"]}"}
-
-  def test_request_parameters_file(self):
-    with mock.patch(
-        'slapos.cli.request.SoftwareReleaseSchema.getSerialisation',
-        return_value='json-in-xml'):
-      super(TestCliRequestParametersFileJsonJsonInXMLSerialisation,
-            self).test_request_parameters_file()
+  serialization = 'json-in-xml'
 
 
 class TestCliRequestParametersFileJsonJsonInXMLSerialisationAlreadySerialised(
-    TestCliRequestParametersFileJson):
+    TestCliRequestParametersFileJsonJsonInXMLSerialisation):
   """Request with --parameter-file, with a .json file and a software using
   json-in-xml for serialisation and parameters already serialised with
   {'_': json.dumps(params)}. In that case, parameters are not serialized one
   more time.
   """
   expected_partition_parameter_kw = {"_": "{\"foo\": [\"bar\"]}"}
-
-  def _makeParameterFile(self):
-    f = tempfile.NamedTemporaryFile(suffix='.json', mode='w', delete=False)
-    self.addCleanup(os.unlink, f.name)
-    f.write(textwrap.dedent(r'''
-      {"_": "{\"foo\": [\"bar\"]}"}
-    '''))
-    f.flush()
-    return f.name
-
-  def test_request_parameters_file(self):
-    with mock.patch(
-        'slapos.cli.request.SoftwareReleaseSchema.getSerialisation',
-        return_value='json-in-xml'):
-      super(
-          TestCliRequestParametersFileJsonJsonInXMLSerialisationAlreadySerialised,
-          self).test_request_parameters_file()
+  parameter_file_content = r'''
+    {"_": "{\"foo\": [\"bar\"]}"}
+  '''
 
 
 class TestCliRequestParametersFileYaml(TestCliRequestParametersFileJson):
   """Request with --parameter-file, with a .yaml file. This behaves like json.
   """
-  def _makeParameterFile(self):
-    f = tempfile.NamedTemporaryFile(suffix='.yaml', mode='w', delete=False)
-    self.addCleanup(os.unlink, f.name)
-    f.write(textwrap.dedent('''\
-      foo:
-      - bar
-    '''))
-    f.flush()
-    return f.name
+  parameter_file_content = '''\
+    foo:
+    - bar
+  '''
+  parameter_file_suffix = '.yaml'
 
 
 class TestCliRequestParametersFileXml(TestCliRequestParametersFileJson):
   """Request with --parameter-file, with a .xml file
   """
   expected_partition_parameter_kw = {'foo': 'bar'}
-  def _makeParameterFile(self):
-    f = tempfile.NamedTemporaryFile(suffix='.xml', mode='w', delete=False)
-    f.write(textwrap.dedent('''\
-      <?xml version="1.0" encoding="utf-8"?>
-      <instance>
-          <parameter id="foo">bar</parameter>
-      </instance>
-    '''))
-    f.flush()
-    self.addCleanup(os.unlink, f.name)
-    return f.name
+  parameter_file_content = '''\
+    <?xml version="1.0" encoding="utf-8"?>
+    <instance>
+        <parameter id="foo">bar</parameter>
+    </instance>
+  '''
+  parameter_file_suffix = '.xml'

slapos/util.py

@@ -70,6 +70,11 @@ _ALLOWED_CLASS_SET = frozenset((
 ))
 
 
+class UndefinedSerializationError(ValueError):
+    """Raised when the serialization type is not found"""
+    pass
+
+
 class SafeXMLMarshaller(Marshaller):
   def m_instance(self, value, kw):
     cls = value.__class__
@@ -393,14 +398,14 @@ class SoftwareReleaseSchema(object):
     return software_schema['software-type'].get(software_type)
 
   def getSerialisation(self):
-    # type: () -> Optional[SoftwareReleaseSerialisation]
+    # type: () -> SoftwareReleaseSerialisation
     """Returns the serialisation method used for parameters.
     """
     software_schema = self.getSoftwareTypeSchema()
     if software_schema is None or 'serialisation' not in software_schema:
       software_schema = self.getSoftwareSchema()
     if software_schema is None:
-      return None
+      raise UndefinedSerializationError
     return SoftwareReleaseSerialisation(software_schema['serialisation'])
 
   def getInstanceRequestParameterSchemaURL(self):