Fix single app deploy not using config_yaml

Refactored to reduce duplicated code and ensure that both deploy paths
(bundle and single app) use new config_yaml param instead of config.

diff --git a/juju/client/connection.py b/juju/client/connection.py
index 77edda5..c24f8c1 100644 (file)
--- a/juju/client/connection.py
+++ b/juju/client/connection.py
@@ -15,7 +15,7 @@ import asyncio
 import yaml
 
 from juju import tag
-from juju.errors import JujuAPIError, JujuConnectionError
+from juju.errors import JujuError, JujuAPIError, JujuConnectionError
 
 log = logging.getLogger("websocket")
 
@@ -221,8 +221,7 @@ class Connection:
         """
         jujudata = JujuData()
         controller_name = jujudata.current_controller()
-        models = jujudata.models()[controller_name]
-        model_name = models['current-model']
+        model_name = jujudata.current_model()
 
         return await cls.connect_model(
             '{}:{}'.format(controller_name, model_name), loop)
@@ -339,6 +338,12 @@ class JujuData:
         output = yaml.safe_load(output)
         return output.get('current-controller', '')
 
+    def current_model(self, controller_name=None):
+        models = self.models()[controller_name or self.current_controller()]
+        if 'current-model' not in models:
+            raise JujuError('No current model')
+        return models['current-model']
+
     def controllers(self):
         return self._load_yaml('controllers.yaml', 'controllers')
 

diff --git a/juju/client/facade.py b/juju/client/facade.py
index 5ed5320..817b37b 100644 (file)
--- a/juju/client/facade.py
+++ b/juju/client/facade.py
@@ -438,6 +438,8 @@ class Type:
 
     @classmethod
     def from_json(cls, data):
+        if isinstance(data, cls):
+            return data
         if isinstance(data, str):
             data = json.loads(data)
         d = {}

diff --git a/juju/model.py b/juju/model.py
index 812f35b..7a52ed5 100644 (file)
--- a/juju/model.py
+++ b/juju/model.py
@@ -974,11 +974,6 @@ class Model(object):
             - series is required; how do we pick a default?
 
         """
-        if to:
-            placement = parse_placement(to)
-        else:
-            placement = []
-
         if storage:
             storage = {
                 k: client.Constraints(**v)
@@ -992,9 +987,7 @@ class Model(object):
         entity_id = await self.charmstore.entityId(entity_url) \
             if not is_local else entity_url
 
-        app_facade = client.ApplicationFacade()
         client_facade = client.ClientFacade()
-        app_facade.connect(self.connection)
         client_facade.connect(self.connection)
 
         is_bundle = ((is_local and
@@ -1019,9 +1012,6 @@ class Model(object):
             return [app for name, app in self.applications.items()
                     if name in handler.applications]
         else:
-            log.debug(
-                'Deploying %s', entity_id)
-
             if not is_local:
                 parts = entity_id[3:].split('/')
                 if parts[0].startswith('~'):
@@ -1047,26 +1037,52 @@ class Model(object):
                         "Pass a 'series' kwarg to Model.deploy().".format(
                             charm_dir))
                 entity_id = await self.add_local_charm_dir(charm_dir, series)
+            return await self._deploy(
+                charm_url=entity_id,
+                application=application_name,
+                series=series,
+                config=config or {},
+                constraints=constraints,
+                endpoint_bindings=bind,
+                resources=resources,
+                storage=storage,
+                channel=channel,
+                num_units=num_units,
+                placement=parse_placement(to),
+            )
+
+    async def _deploy(self, charm_url, application, series, config,
+                      constraints, endpoint_bindings, resources, storage,
+                      channel=None, num_units=None, placement=None):
+            log.info('Deploying %s', charm_url)
+
+            # stringify all config values for API, and convert to YAML
+            config = {k: str(v) for k, v in config.items()}
+            config = yaml.dump({application: config},
+                               default_flow_style=False)
+
+            app_facade = client.ApplicationFacade()
+            app_facade.connect(self.connection)
 
             app = client.ApplicationDeploy(
-                application=application_name,
+                charm_url=charm_url,
+                application=application,
+                series=series,
                 channel=channel,
-                charm_url=entity_id,
-                config=config,
+                config_yaml=config,
                 constraints=parse_constraints(constraints),
-                endpoint_bindings=bind,
+                endpoint_bindings=endpoint_bindings,
                 num_units=num_units,
                 resources=resources,
-                series=series,
                 storage=storage,
+                placement=placement,
             )
-            app.placement = placement
 
             result = await app_facade.Deploy([app])
             errors = [r.error.message for r in result.results if r.error]
             if errors:
                 raise JujuError('\n'.join(errors))
-            return await self._wait_for_new('application', application_name)
+            return await self._wait_for_new('application', application)
 
     def destroy(self):
         """Terminate all machines and resources for this model.
@@ -1672,28 +1688,16 @@ class BundleHandler(object):
         """
         # resolve indirect references
         charm = self.resolve(charm)
-        # stringify all config values for API, and convert to YAML
-        options = {k: str(v) for k, v in options.items()}
-        options = yaml.dump({application: options}, default_flow_style=False)
-        # build param object
-        app = client.ApplicationDeploy(
+        await self.model._deploy(
             charm_url=charm,
-            series=series,
             application=application,
-            # Pass options to config-yaml rather than config, as
-            # config-yaml invokes a newer codepath that better handles
-            # empty strings in the options values.
-            config_yaml=options,
-            constraints=parse_constraints(constraints),
-            storage=storage,
+            series=series,
+            config=options,
+            constraints=constraints,
             endpoint_bindings=endpoint_bindings,
             resources=resources,
+            storage=storage,
         )
-        # do the do
-        log.info('Deploying %s', charm)
-        await self.app_facade.Deploy([app])
-        # ensure the app is in the model for future operations
-        await self.model._wait_for_new('application', application)
         return application
 
     async def addUnit(self, application, to):

diff --git a/tests/base.py b/tests/base.py
index 382da43..af386ea 100644 (file)
--- a/tests/base.py
+++ b/tests/base.py
@@ -1,9 +1,11 @@
-import uuid
+import mock
 import subprocess
+import uuid
 
 import pytest
 
 from juju.controller import Controller
+from juju.client.connection import JujuData
 
 
 def is_bootstrapped():
@@ -29,9 +31,16 @@ class CleanModel():
         model_name = 'model-{}'.format(uuid.uuid4())
         self.model = await self.controller.add_model(model_name)
 
+        # Ensure that we connect to the new model by default.  This also
+        # prevents failures if test was started with no current model.
+        self._patch_cm = mock.patch.object(JujuData, 'current_model',
+                                           return_value=model_name)
+        self._patch_cm.start()
+
         return self.model
 
     async def __aexit__(self, exc_type, exc, tb):
+        self._patch_cm.stop()
         await self.model.disconnect()
         await self.controller.destroy_model(self.model.info.uuid)
         await self.controller.disconnect()