Repository: dwavesystems/dwave-cloud-client

Issue:
Compatibility break in from_config for None kwargs
Prior to 0.8.0, kwargs with value `None` did not overwrite the file/env config.

We should restore that as a default behavior, as it easily propagates up to libraries that call `Client.from_config(**kwargs)`.

Discussion:

Patch:
diff --git a/dwave/cloud/client.py b/dwave/cloud/client.py
index abace57..baf8341 100644
--- a/dwave/cloud/client.py
+++ b/dwave/cloud/client.py
@@ -283,7 +283,8 @@ class Client(object):
         logger.debug("File/env config loaded: %r", config)
 
         # manual config override with client constructor options
-        config.update(client=client, **kwargs)
+        kwargs.update(client=client)
+        config.update({k: v for k, v in kwargs.items() if v is not None})
         logger.debug("Code config loaded: %r", config)
 
         from dwave.cloud import qpu, sw, hybrid
diff --git a/dwave/cloud/config.py b/dwave/cloud/config.py
index a8418ed..455f086 100644
--- a/dwave/cloud/config.py
+++ b/dwave/cloud/config.py
@@ -754,7 +754,12 @@ def load_config(config_file=None, profile=None, **kwargs):
         :exc:`~dwave.cloud.exceptions.ConfigFileParseError`:
             Config file parse failed.
 
-    Examples
+    Note:
+        Prior to 0.8.0, some keyword arguments did not overwrite config
+        variables when their value was ``None``. Now we consistently do
+        :meth:`dict.update` on the config read from file/env for all ``kwargs``.
+
+    Examples:
         This example loads the configuration from an auto-detected configuration file
         in the home directory of a Windows system user.
 


Test patch:
diff --git a/tests/test_client.py b/tests/test_client.py
index bc6717a..6c468c2 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -28,7 +28,6 @@ from contextlib import contextmanager
 import requests.exceptions
 from plucky import merge
 
-from dwave.cloud.config import load_config
 from dwave.cloud.client import Client
 from dwave.cloud.solver import StructuredSolver, UnstructuredSolver
 from dwave.cloud.exceptions import (
@@ -174,7 +173,7 @@ class ClientFactory(unittest.TestCase):
 
     def test_default(self):
         conf = {k: k for k in 'endpoint token'.split()}
-        with mock.patch("dwave.cloud.client.load_config", lambda **kwargs: conf):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config() as client:
                 self.assertEqual(client.endpoint, 'endpoint')
                 self.assertEqual(client.token, 'token')
@@ -185,11 +184,8 @@ class ClientFactory(unittest.TestCase):
 
     def test_client_type(self):
         conf = {k: k for k in 'endpoint token'.split()}
-        def mocked_load_config(**kwargs):
-            kwargs.update(conf)
-            return kwargs
 
-        with mock.patch("dwave.cloud.client.load_config", mocked_load_config):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config() as client:
                 self.assertIsInstance(client, dwave.cloud.client.Client)
 
@@ -219,7 +215,7 @@ class ClientFactory(unittest.TestCase):
 
     def test_custom_kwargs(self):
         conf = {k: k for k in 'endpoint token'.split()}
-        with mock.patch("dwave.cloud.client.load_config", lambda **kwargs: conf):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with mock.patch("dwave.cloud.client.Client.__init__", return_value=None) as init:
                 dwave.cloud.Client.from_config(custom='custom')
                 init.assert_called_once_with(
@@ -227,7 +223,7 @@ class ClientFactory(unittest.TestCase):
 
     def test_custom_kwargs_overrides_config(self):
         conf = {k: k for k in 'endpoint token custom'.split()}
-        with mock.patch("dwave.cloud.client.load_config", lambda **kwargs: conf):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with mock.patch("dwave.cloud.client.Client.__init__", return_value=None) as init:
                 dwave.cloud.Client.from_config(custom='new-custom')
                 init.assert_called_once_with(
@@ -238,7 +234,7 @@ class ClientFactory(unittest.TestCase):
         conf = {k: k for k in 'endpoint token'.split()}
         conf.update(solver=json.dumps(solver_def))
 
-        with mock.patch("dwave.cloud.client.load_config", lambda **kwargs: conf):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config() as client:
                 self.assertEqual(client.default_solver, solver_def)
 
@@ -246,7 +242,7 @@ class ClientFactory(unittest.TestCase):
         solver_def = {"name__eq": "solver"}
         conf = {k: k for k in 'endpoint token solver'.split()}
 
-        with mock.patch("dwave.cloud.client.load_config", lambda **kwargs: conf):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config() as client:
                 self.assertEqual(client.default_solver, solver_def)
 
@@ -254,28 +250,34 @@ class ClientFactory(unittest.TestCase):
         new_solver_def = {"software": True}
         conf = {k: k for k in 'endpoint token solver'.split()}
 
-        def load_config(**kwargs):
-            return merge(kwargs, conf, op=lambda a, b: a or b)
-
-        with mock.patch("dwave.cloud.client.load_config", load_config):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config(solver=new_solver_def) as client:
                 self.assertEqual(client.default_solver, new_solver_def)
 
+    def test_none_kwargs_do_not_override_config(self):
+        """kwargs with value ``None`` should be ignored (issue #430)"""
+        conf = {k: k for k in 'endpoint token'.split()}
+        solver_json = '{"qpu": true}'
+        conf.update(solver=solver_json)
+        solver = json.loads(solver_json)
+
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
+            with dwave.cloud.Client.from_config(endpoint=None, solver=None) as client:
+                self.assertEqual(client.endpoint, conf['endpoint'])
+                self.assertEqual(client.default_solver, solver)
+
     def test_solver_name_overrides_config_features(self):
         conf = {k: k for k in 'endpoint token solver'.split()}
         conf.update(solver=json.dumps({"software": True}))
 
-        def load_config(**kwargs):
-            return merge(kwargs, conf, op=lambda a, b: a or b)
-
-        with mock.patch("dwave.cloud.client.load_config", load_config):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config(solver='solver') as client:
                 self.assertEqual(client.default_solver, {"name__eq": "solver"})
 
     def test_boolean_options_parsed_from_config(self):
         conf = {'connection_close': 'off', 'permissive_ssl': 'true'}
 
-        with mock.patch("dwave.cloud.client.load_config", lambda **kwargs: conf):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config(token='token') as client:
                 self.assertFalse(client.connection_close)
                 self.assertTrue(client.permissive_ssl)
@@ -285,16 +287,32 @@ class ClientFactory(unittest.TestCase):
         DEFAULTS = Client.DEFAULTS.copy()
         DEFAULTS.update(token=token)
 
-        with mock.patch("dwave.cloud.client.load_config", lambda **kwargs: {}):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: {}):
             with mock.patch.multiple("dwave.cloud.Client", DEFAULTS=DEFAULTS):
                 with dwave.cloud.Client.from_config() as client:
                     self.assertEqual(client.token, token)
 
+                # None defaults are ignored
+                with dwave.cloud.Client(defaults=None) as client:
+                    self.assertEqual(client.token, token)
+
+                # explicit None kwargs do not modify defaults
+                with dwave.cloud.Client(
+                        endpoint=None, token=None, solver=None,
+                        connection_close=None, poll_backoff_min=None) as client:
+
+                    self.assertEqual(client.endpoint, client.DEFAULT_API_ENDPOINT)
+                    self.assertEqual(client.token, token)
+                    self.assertEqual(client.default_solver, {})
+
+                    self.assertEqual(client.connection_close, DEFAULTS['connection_close'])
+                    self.assertEqual(client.poll_backoff_min, DEFAULTS['poll_backoff_min'])
+
     def test_defaults_as_kwarg(self):
         token = 'value'
         defaults = dict(token=token)
 
-        with mock.patch("dwave.cloud.client.load_config", lambda **kwargs: {}):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: {}):
             with dwave.cloud.Client.from_config(defaults=defaults) as client:
                 self.assertEqual(client.token, token)
 
@@ -303,19 +321,18 @@ class ClientFactory(unittest.TestCase):
 
         token = 'value'
         solver = {'feature': 'value'}
+        request_timeout = 10
 
         DEFAULTS = Client.DEFAULTS.copy()
         DEFAULTS.update(token='wrong')
 
         defaults = dict(solver='wrong')
 
-        conf = dict(solver=solver)
-        def load_config(**kwargs):
-            return merge(kwargs, conf, op=lambda a, b: a or b)
+        conf = dict(solver=solver, request_timeout=request_timeout)
 
-        kwargs = dict(token=token, defaults=defaults)
+        kwargs = dict(token=token, defaults=defaults, request_timeout=None)
 
-        with mock.patch("dwave.cloud.client.load_config", load_config):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with mock.patch.multiple("dwave.cloud.Client", DEFAULTS=DEFAULTS):
                 with dwave.cloud.Client.from_config(**kwargs) as client:
 
@@ -328,6 +345,9 @@ class ClientFactory(unittest.TestCase):
                     # endpoint: used from class defaults
                     self.assertEqual(client.endpoint, DEFAULTS['endpoint'])
 
+                    # None kwarg: used from class defaults
+                    self.assertEqual(client.request_timeout, request_timeout)
+
     def test_headers_from_config(self):
         headers_dict = {"key-1": "value-1", "key-2": "value-2"}
         headers_str = """  key-1:value-1
@@ -335,7 +355,7 @@ class ClientFactory(unittest.TestCase):
         """
         conf = dict(token='token', headers=headers_str)
 
-        with mock.patch("dwave.cloud.client.load_config", lambda **kwargs: conf):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config() as client:
                 self.assertDictEqual(client.headers, headers_dict)
 
@@ -344,16 +364,13 @@ class ClientFactory(unittest.TestCase):
         headers_str = "key-2:value-2\nkey-1:value-1"
         conf = dict(token='token')
 
-        def load_config(**kwargs):
-            return merge(kwargs, conf, op=lambda a, b: a or b)
-
         # headers as dict
-        with mock.patch("dwave.cloud.client.load_config", load_config):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config(headers=headers_dict) as client:
                 self.assertDictEqual(client.headers, headers_dict)
 
         # headers as str
-        with mock.patch("dwave.cloud.client.load_config", load_config):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config(headers=headers_str) as client:
                 self.assertDictEqual(client.headers, headers_dict)
 
@@ -365,7 +382,7 @@ class ClientFactory(unittest.TestCase):
         client_cert = crt
         conf = dict(token='token', client_cert=crt)
 
-        with mock.patch("dwave.cloud.client.load_config", lambda **kwargs: conf):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config() as client:
                 self.assertEqual(client.client_cert, client_cert)
 
@@ -376,7 +393,7 @@ class ClientFactory(unittest.TestCase):
         client_cert = (crt, key)
         conf = dict(token='token', client_cert=crt, client_cert_key=key)
 
-        with mock.patch("dwave.cloud.client.load_config", lambda **kwargs: conf):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config() as client:
                 self.assertEqual(client.client_cert, client_cert)
 
@@ -420,14 +437,14 @@ class ClientFactory(unittest.TestCase):
         conf = dict(token='token', **poll_conf)
 
         # polling params from config file propagated to client object
-        with mock.patch("dwave.cloud.client.load_config", lambda **kwargs: conf):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config() as client:
                 self.assertEqual(client.poll_backoff_min, 0.1)
                 self.assertEqual(client.poll_backoff_max, 1.0)
 
         # test defaults
         conf = dict(token='token')
-        with mock.patch("dwave.cloud.client.load_config", lambda **kwargs: conf):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config() as client:
                 self.assertEqual(client.poll_backoff_min, Client.DEFAULTS['poll_backoff_min'])
                 self.assertEqual(client.poll_backoff_max, Client.DEFAULTS['poll_backoff_max'])
@@ -436,10 +453,7 @@ class ClientFactory(unittest.TestCase):
         poll_conf = {"poll_backoff_min": "0.1", "poll_backoff_max": "1"}
         conf = dict(token='token', **poll_conf)
 
-        def load_config(**kwargs):
-            return merge(kwargs, conf, op=lambda a, b: a or b)
-
-        with mock.patch("dwave.cloud.client.load_config", load_config):
+        with mock.patch("dwave.cloud.client.load_config", lambda **kw: conf):
             with dwave.cloud.Client.from_config(poll_backoff_min=0.5) as client:
                 self.assertEqual(client.poll_backoff_min, 0.5)
                 self.assertEqual(client.poll_backoff_max, 1.0)