Merge "Coerce port to integer"

Author: zzzeek

sqlalchemy_collectd/client/plugin.py

@@ -18,7 +18,7 @@ def __init__(self, url, kwargs):
             "collectd_host", "collectd_host", self.config, url, kwargs
         )
         self._get_argument(
-            "collectd_port", "collectd_port", self.config, url, kwargs
+            "collectd_port", "collectd_port", self.config, url, kwargs, int
         )
         self._get_argument(
             "collectd_report_host", "hostname", self.config, url, kwargs
@@ -27,13 +27,17 @@ def __init__(self, url, kwargs):
             "collectd_program_name", "progname", self.config, url, kwargs
         )
 
-    def _get_argument(self, name, dest_name, dest, url, kwargs):
+    def _get_argument(self, name, dest_name, dest, url, kwargs, coerce_=None):
         # favor the URL but pop the name from both
         if name in kwargs:
             dest[dest_name] = kwargs.pop(name)
+            if coerce_:
+                dest[dest_name] = coerce_(dest[dest_name])
 
         if name in url.query:
             dest[dest_name] = url.query.pop(name)
+            if coerce_:
+                dest[dest_name] = coerce_(dest[dest_name])
 
     def handle_dialect_kwargs(self, dialect_cls, dialect_args):
         """parse and modify dialect kwargs"""

sqlalchemy_collectd/client/tests/test_plugin.py

@@ -19,16 +19,36 @@ def test_start_no_args(self):
     def test_start_engine_args(self):
         with mock.patch.object(plugin, "start_plugin") as start_plugin:
             url = sqla_url.URL("mysql+pymysql://scott:tiger@localhost/")
-            p = plugin.Plugin(url, {"collectd_host": "127.0.0.1"})
+            p = plugin.Plugin(
+                url, {"collectd_host": "127.0.0.1", "collectd_port": 5678}
+            )
             engine = mock.Mock()
             p.engine_created(engine)
 
         self.assertEqual(
-            [mock.call(engine, collectd_host="127.0.0.1")],
+            [mock.call(engine, collectd_host="127.0.0.1", collectd_port=5678)],
             start_plugin.mock_calls,
         )
 
     def test_start_url_args(self):
+        with mock.patch.object(plugin, "start_plugin") as start_plugin:
+            url = sqla_url.make_url(
+                "mysql+pymysql://scott:tiger@localhost/"
+                "?collectd_host=127.0.0.1&somekey=somevalue&collectd_port=1234"
+            )
+            kwargs = {"unrelated": "bar"}
+            p = plugin.Plugin(url, kwargs)
+            engine = mock.Mock()
+            p.engine_created(engine)
+
+        self.assertEqual(
+            [mock.call(engine, collectd_host="127.0.0.1", collectd_port=1234)],
+            start_plugin.mock_calls,
+        )
+        self.assertEqual({"somekey": "somevalue"}, url.query)
+        self.assertEqual({"unrelated": "bar"}, kwargs)
+
+    def test_start_url_args_no_port(self):
         with mock.patch.object(plugin, "start_plugin") as start_plugin:
             url = sqla_url.make_url(
                 "mysql+pymysql://scott:tiger@localhost/"
@@ -50,16 +70,16 @@ def test_start_both_args(self):
         with mock.patch.object(plugin, "start_plugin") as start_plugin:
             url = sqla_url.make_url(
                 "mysql+pymysql://scott:tiger@localhost/"
-                "?collectd_host=127.0.0.1"
+                "?collectd_host=127.0.0.1&collectd_port=1234"
             )
-            kwargs = {"collectd_host": "172.18.0.2"}
+            kwargs = {"collectd_host": "172.18.0.2", "collectd_port": 5678}
             p = plugin.Plugin(url, kwargs)
             engine = mock.Mock()
             p.engine_created(engine)
 
         # argument is popped from both but favors url argument
         self.assertEqual(
-            [mock.call(engine, collectd_host="127.0.0.1")],
+            [mock.call(engine, collectd_host="127.0.0.1", collectd_port=1234)],
             start_plugin.mock_calls,
         )
         self.assertEqual({}, url.query)

unreleased_changes/3.rst

@@ -0,0 +1,7 @@
+.. change::
+    :tags: bug, config
+    :tickets: 3
+
+    Fixed bug where the port number included in the SQLAlchemy URL with the
+    collectd_port query string value would not be coerced into an integer,
+    failing when it is passed to the socket send operation.