Treat API_HOST as URL (#419)

* Remove trailing whitespace from api_host * Attempt to fix url issue * Implement first review feedback * Better understanding of urljoin behavior * Do not use the API url in tests
DataDog · Aug 16, 2019 · 42b578c · 42b578c
1 parent 26b9343
commit 42b578c
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 24 deletions.
diff --git a/datadog/api/api_client.py b/datadog/api/api_client.py
@@ -13,7 +13,8 @@
     ApiNotInitialized
 )
 from datadog.api.http_client import resolve_http_client
-from datadog.util.compat import is_p3k
+from datadog.util.compat import is_p3k, urljoin
+
 
 log = logging.getLogger('datadog.api')
 
@@ -117,12 +118,10 @@ def submit(cls, method, path, api_version=None, body=None, attach_host_name=Fals
             if isinstance(body, dict):
                 body = json.dumps(body)
                 headers['Content-Type'] = 'application/json'
+
             # Construct the URL
-            url = "{api_host}/api/{api_version}/{path}".format(
-                  api_host=_api_host,
-                  api_version=api_version,
-                  path=path.lstrip("/"),
-            )
+            start_url = urljoin(_api_host, 'api/{}/'.format(api_version))
+            url = urljoin(start_url, path)
 
             # Process requesting
             start_time = time.time()

diff --git a/datadog/util/compat.py b/datadog/util/compat.py
@@ -7,6 +7,11 @@
 import socket
 import sys
 
+try:
+    from urllib.parse import urljoin
+except ImportError:
+    from urlparse import urljoin
+
 
 def _is_py_version_higher_than(major, minor=0):
     """

diff --git a/tests/unit/api/helper.py b/tests/unit/api/helper.py
@@ -28,7 +28,7 @@
 
 API_KEY = "apikey"
 APP_KEY = "applicationkey"
-API_HOST = "host"
+API_HOST = "https://example.com"
 HOST_NAME = "agent.hostname"
 FAKE_PROXY = {
     "https": "http://user:pass@10.10.1.10:3128/",

diff --git a/tests/unit/api/test_api.py b/tests/unit/api/test_api.py
@@ -240,10 +240,10 @@ def test_creatable(self):
         Creatable resource logic.
         """
         MyCreatable.create(mydata="val")
-        self.request_called_with('POST', "host/api/v1/creatables", data={'mydata': "val"})
+        self.request_called_with('POST', API_HOST + "/api/v1/creatables", data={'mydata': "val"})
 
         MyCreatable.create(mydata="val", attach_host_name=True)
-        self.request_called_with('POST', "host/api/v1/creatables",
+        self.request_called_with('POST', API_HOST + "/api/v1/creatables",
                                  data={'mydata': "val", 'host': api._host_name})
 
     def test_getable(self):
@@ -252,7 +252,7 @@ def test_getable(self):
         """
         getable_object_id = 123
         MyGetable.get(getable_object_id, otherparam="val")
-        self.request_called_with('GET', "host/api/v1/getables/" + str(getable_object_id),
+        self.request_called_with('GET', API_HOST + "/api/v1/getables/" + str(getable_object_id),
                                  params={'otherparam': "val"})
         _, kwargs = self.request_mock.call_args()
         self.assertIsNone(kwargs["data"])
@@ -262,7 +262,7 @@ def test_listable(self):
         Listable resource logic.
         """
         MyListable.get_all(otherparam="val")
-        self.request_called_with('GET', "host/api/v1/listables", params={'otherparam': "val"})
+        self.request_called_with('GET', API_HOST + "/api/v1/listables", params={'otherparam': "val"})
         _, kwargs = self.request_mock.call_args()
         self.assertIsNone(kwargs["data"])
 
@@ -272,7 +272,7 @@ def test_updatable(self):
         """
         updatable_object_id = 123
         MyUpdatable.update(updatable_object_id, params={'myparam': "val1"}, mydata="val2")
-        self.request_called_with('PUT', "host/api/v1/updatables/" + str(updatable_object_id),
+        self.request_called_with('PUT', API_HOST + "/api/v1/updatables/" + str(updatable_object_id),
                                  params={'myparam': "val1"}, data={'mydata': "val2"})
 
     def test_detalable(self):
@@ -281,7 +281,7 @@ def test_detalable(self):
         """
         deletable_object_id = 123
         MyDeletable.delete(deletable_object_id, otherparam="val")
-        self.request_called_with('DELETE', "host/api/v1/deletables/" + str(deletable_object_id),
+        self.request_called_with('DELETE', API_HOST + "/api/v1/deletables/" + str(deletable_object_id),
                                  params={'otherparam': "val"})
 
     def test_listable_sub_resources(self):
@@ -292,7 +292,7 @@ def test_listable_sub_resources(self):
         MyListableSubResource.get_items(resource_id, otherparam="val")
         self.request_called_with(
             'GET',
-            'host/api/v1/resource_name/{0}/sub_resource_name'.format(resource_id),
+            API_HOST + '/api/v1/resource_name/{0}/sub_resource_name'.format(resource_id),
             params={'otherparam': "val"}
         )
         _, kwargs = self.request_mock.call_args()
@@ -306,7 +306,7 @@ def test_addable_sub_resources(self):
         MyAddableSubResource.add_items(resource_id, params={'myparam': 'val1'}, mydata='val2')
         self.request_called_with(
             'POST',
-            'host/api/v1/resource_name/{0}/sub_resource_name'.format(resource_id),
+            API_HOST + '/api/v1/resource_name/{0}/sub_resource_name'.format(resource_id),
             params={'myparam': 'val1'},
             data={'mydata': 'val2'}
         )
@@ -319,7 +319,7 @@ def test_updatable_sub_resources(self):
         MyUpdatableSubResource.update_items(resource_id, params={'myparam': 'val1'}, mydata='val2')
         self.request_called_with(
             'PUT',
-            'host/api/v1/resource_name/{0}/sub_resource_name'.format(resource_id),
+            API_HOST + '/api/v1/resource_name/{0}/sub_resource_name'.format(resource_id),
             params={'myparam': 'val1'},
             data={'mydata': 'val2'}
         )
@@ -332,7 +332,7 @@ def test_deletable_sub_resources(self):
         MyDeletableSubResource.delete_items(resource_id, params={'myparam': 'val1'}, mydata='val2')
         self.request_called_with(
             'DELETE',
-            'host/api/v1/resource_name/{0}/sub_resource_name'.format(resource_id),
+            API_HOST + '/api/v1/resource_name/{0}/sub_resource_name'.format(resource_id),
             params={'myparam': 'val1'},
             data={'mydata': 'val2'}
         )
@@ -352,7 +352,7 @@ def test_actionable(self):
         )
         self.request_called_with(
             'POST',
-            'host/api/v1/actionables/{0}/actionname'.format(str(actionable_object_id)),
+            API_HOST + '/api/v1/actionables/{0}/actionname'.format(str(actionable_object_id)),
             params={'myparam': 'val1'},
             data={'mydata': 'val', 'mydata2': 'val2'}
         )
@@ -366,7 +366,7 @@ def test_actionable(self):
         )
         self.request_called_with(
             'POST',
-            'host/api/v1/actionables/{0}/actionname'.format(str(actionable_object_id)),
+            API_HOST +'/api/v1/actionables/{0}/actionname'.format(str(actionable_object_id)),
             params={},
             data={'mydata': 'val', 'mydata2': 'val2'}
         )
@@ -379,7 +379,7 @@ def test_actionable(self):
         )
         self.request_called_with(
             'GET',
-            'host/api/v1/actionables/{0}/actionname'.format(str(actionable_object_id)),
+            API_HOST + '/api/v1/actionables/{0}/actionname'.format(str(actionable_object_id)),
             params={'param1': 'val1', 'param2': 'val2'}
         )
         _, kwargs = self.request_mock.call_args()
@@ -393,7 +393,7 @@ def test_actionable(self):
         )
         self.request_called_with(
             'POST',
-            'host/api/v1/actionname/{0}'.format(actionable_object_id),
+            API_HOST + '/api/v1/actionname/{0}'.format(actionable_object_id),
             data={'mydata': "val"}
         )
 
@@ -404,7 +404,7 @@ def test_actionable(self):
         )
         self.request_called_with(
             'GET',
-            'host/api/v1/actionname/{0}'.format(actionable_object_id)
+            API_HOST + '/api/v1/actionname/{0}'.format(actionable_object_id)
         )
         _, kwargs = self.request_mock.call_args()
         self.assertIsNone(kwargs["data"])
@@ -483,11 +483,11 @@ def test_metric_submit_query_switch(self):
         Endpoints are different for submission and queries.
         """
         Metric.send(points=(123, 456))
-        self.request_called_with('POST', "host/api/v1/series",
+        self.request_called_with('POST', API_HOST + "/api/v1/series",
                                  data={'series': [{'points': [[123, 456.0]], 'host': api._host_name}]})
 
         Metric.query(start="val1", end="val2")
-        self.request_called_with('GET', "host/api/v1/query",
+        self.request_called_with('GET', API_HOST + "/api/v1/query",
                                  params={'from': "val1", 'to': "val2"})
 
     def test_points_submission(self):