fix: only append kwargs as query parameters

bbatliner · bbatliner · commit b9ecc9a8c5d9 · 2021-04-07T12:05:12.000-05:00
Some arguments to `http_request` were being read
from kwargs, but kwargs is where this function
creates query parameters from, by default. In
the absence of a `query_parameters` param, the
function would construct URLs with query
parameters such as `retry_transient_errors=True`
despite those parameters having no meaning to
the API to which the request was sent.

This change names those arguments that are
specific to `http_request` so that they do not
end up as query parameters read from kwargs.
diff --git a/gitlab/client.py b/gitlab/client.py
@@ -451,6 +451,10 @@ def http_request(
         post_data: Optional[Dict[str, Any]] = None,
         streamed: bool = False,
         files: Optional[Dict[str, Any]] = None,
+        timeout: Optional[float] = None,
+        obey_rate_limit: bool = True,
+        retry_transient_errors: bool = False,
+        max_retries: int = 10,
         **kwargs: Any,
     ) -> requests.Response:
         """Make an HTTP request to the Gitlab server.
@@ -465,6 +469,14 @@ def http_request(
                               json)
             streamed (bool): Whether the data should be streamed
             files (dict): The files to send to the server
+            timeout (float): The timeout, in seconds, for the request
+            obey_rate_limit (bool): Whether to obey 429 Too Many Request
+                                    responses. Defaults to True.
+            retry_transient_errors (bool): Whether to retry after 500, 502,
+                                           503, or 504 responses. Defaults
+                                           to False.
+            max_retries (int): Max retries after 429 or transient errors,
+                               set to -1 to retry forever. Defaults to 10.
             **kwargs: Extra options to send to the server (e.g. sudo)
 
         Returns:
@@ -496,9 +508,10 @@ def http_request(
         opts = self._get_session_opts(content_type="application/json")
 
         verify = opts.pop("verify")
-        timeout = opts.pop("timeout")
+        opts_timeout = opts.pop("timeout")
         # If timeout was passed into kwargs, allow it to override the default
-        timeout = kwargs.get("timeout", timeout)
+        if timeout is None:
+            timeout = opts_timeout
 
         # We need to deal with json vs. data when uploading files
         if files:
@@ -526,15 +539,7 @@ def http_request(
             prepped.url, {}, streamed, verify, None
         )
 
-        # obey the rate limit by default
-        obey_rate_limit = kwargs.get("obey_rate_limit", True)
-        # do not retry transient errors by default
-        retry_transient_errors = kwargs.get("retry_transient_errors", False)
-
-        # set max_retries to 10 by default, disable by setting it to -1
-        max_retries = kwargs.get("max_retries", 10)
         cur_retries = 0
-
         while True:
             result = self.session.send(prepped, timeout=timeout, **settings)
 
