Repository: opensearch-project/opensearch-py (python)

Issue:
[BUG] Heathcheck timeout parameter not parsable
### What is the bug?

the OpenSearch client cannot change the default timeout for health checks.



The cluster endpoint for healthcheck requires a string representing time units:

```
http "os-morel-1:9200/_cluster/health?timeout=4s"
```

It fails if you supply an integer or float.

```
(rayish) √ deus_lex % http "os-morel-1:9200/_cluster/health?timeout=4.2"
HTTP/1.1 400 Bad Request
content-encoding: gzip
content-length: 172
content-type: application/json; charset=UTF-8

{
    "error": {
        "reason": "failed to parse setting [timeout] with value [4.2] as a time value: unit is missing or unrecognized",
        "root_cause": [
            {
                "reason": "failed to parse setting [timeout] with value [4.2] as a time value: unit is missing or unrecognized",
                "type": "illegal_argument_exception"
            }
        ],
        "type": "illegal_argument_exception"
    },
    "status": 400
}
```

However, the client call requires a numeric value and fails with a time unit string

```
x = OpenSearch(["http://os-morel-1:9200"])

x.cluster.health(timeout="4s")

ConnectionError: ConnectionError(Timeout value connect was b'4s', but it must be an int, float or None.) caused by: ValueError(Timeout value connect was b'4s', but it must be an int, float or None.)

x.cluster.health(timeout=4)

ConnectionError: ConnectionError(Timeout value connect was 4, but it must be an int, float or None.) caused by: ValueError(Timeout value connect was 4, but it must be an int, float or None.)
```

### How can one reproduce the bug?
_Steps to reproduce the behavior._

### What is the expected behavior?

As with previous versions of elasticsearch client, I expect "timeout="4s" to work properly

### What is your host/environment?
_Operating system, version._

### Do you have any screenshots?
_If applicable, add screenshots to help explain your problem._

### Do you have any additional context?
_Add any other context about the problem._


Pull request:
[#387] Do not escape the "timeout" parameter.
When the "timeout" parameter is escaped and turned into a string, it does not pass validation checks.
This PR ensures that timeout is passed through as-is, just like "request_timeout".
There's no explicit test for this parameter, but I included it on some existing tests just to ensure that the code is exercised.  Without the fix in place, one of the tests as modified will not pass.

### Description
With this change, using the `timeout` parameter in `cluster.health` no longer throws a validation error.  I have not checked whether the problem exists elsewhere.

### Issues Resolved
Resolves #387 

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check [here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5e3b6774..2ca569f1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Compatibility with OpenSearch 2.1.0 - 2.6.0 ([#381](https://github.com/opensearch-project/opensearch-py/pull/381))
 ### Changed
 - Upgrading pytest-asyncio to latest version - 0.21.0 ([#339](https://github.com/opensearch-project/opensearch-py/pull/339))
+- Fixed flaky CI tests by replacing httpbin with a simple http_server ([#395](https://github.com/opensearch-project/opensearch-py/pull/395))
 ### Deprecated
 ### Removed
 ### Fixed
@@ -72,6 +73,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Updates `master` to `cluster_manager` to be inclusive ([#242](https://github.com/opensearch-project/opensearch-py/pull/242))
 - Support a custom signing service name for AWS SigV4 ([#268](https://github.com/opensearch-project/opensearch-py/pull/268))
 - Updated CI tests to make them work locally ([#275](https://github.com/opensearch-project/opensearch-py/pull/275))
+- Fix bug with validation of 'timeout' parameter ([#387](Do not escape the "timeout" parameter.))
 ### Deprecated
 
 ### Removed
@@ -85,4 +87,5 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 [2.0.1]: https://github.com/opensearch-project/opensearch-py/compare/v2.0.0...v2.0.1
 [2.1.0]: https://github.com/opensearch-project/opensearch-py/compare/v2.0.1...v2.1.0
 [2.1.1]: https://github.com/opensearch-project/opensearch-py/compare/v2.1.0...v2.1.1
-[2.2.0]: https://github.com/opensearch-project/opensearch-py/compare/v2.1.1...v2.2.0
\ No newline at end of file
+[2.2.0]: https://github.com/opensearch-project/opensearch-py/compare/v2.1.1...v2.2.0
+
diff --git a/opensearchpy/client/utils.py b/opensearchpy/client/utils.py
index e79b6e9e..a5e99b11 100644
--- a/opensearchpy/client/utils.py
+++ b/opensearchpy/client/utils.py
@@ -165,16 +165,17 @@ def query_params(*opensearch_query_params):
             elif api_key is not None:
                 headers["authorization"] = "ApiKey %s" % (_base64_auth_header(api_key),)
 
+            # don't escape ignore, request_timeout, or timeout
+            for p in ("ignore", "request_timeout", "timeout"):
+                if p in kwargs:
+                    params[p] = kwargs.pop(p)
+
             for p in opensearch_query_params + GLOBAL_PARAMS:
                 if p in kwargs:
                     v = kwargs.pop(p)
                     if v is not None:
                         params[p] = _escape(v)
 
-            # don't treat ignore, request_timeout, and opaque_id as other params to avoid escaping
-            for p in ("ignore", "request_timeout"):
-                if p in kwargs:
-                    params[p] = kwargs.pop(p)
             return func(*args, params=params, headers=headers, **kwargs)
 
         return _wrapped


Test patch:
diff --git a/test_opensearchpy/TestHttpServer.py b/test_opensearchpy/TestHttpServer.py
new file mode 100644
index 00000000..4c35f621
--- /dev/null
+++ b/test_opensearchpy/TestHttpServer.py
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: Apache-2.0
+#
+# The OpenSearch Contributors require contributions made to
+# this file be licensed under the Apache-2.0 license or a
+# compatible open source license.
+#
+# Modifications Copyright OpenSearch Contributors. See
+# GitHub history for details.
+
+import json
+import threading
+from http.server import BaseHTTPRequestHandler, HTTPServer
+
+
+class TestHTTPRequestHandler(BaseHTTPRequestHandler):
+    def do_GET(self):
+        self.send_response(200)
+        headers = self.headers
+        self.send_header("Content-type", "application/json")
+        self.end_headers()
+
+        Headers = {}
+        for header, value in headers.items():
+            capitalized_header = "-".join([word.title() for word in header.split("-")])
+            Headers.update({capitalized_header: value})
+        if "Connection" in Headers:
+            Headers.pop("Connection")
+
+        data = {"method": "GET", "headers": Headers}
+        self.wfile.write(json.dumps(data).encode("utf-8"))
+
+
+class TestHTTPServer(HTTPServer):
+    def __init__(self, host="localhost", port=8080):
+        super().__init__((host, port), TestHTTPRequestHandler)
+        self._server_thread = None
+
+    def start(self):
+        if self._server_thread is not None:
+            return
+
+        self._server_thread = threading.Thread(target=self.serve_forever)
+        self._server_thread.start()
+
+    def stop(self):
+        if self._server_thread is None:
+            return
+        self.socket.close()
+        self.shutdown()
+        self._server_thread.join()
+        self._server_thread = None
diff --git a/test_opensearchpy/test_async/test_connection.py b/test_opensearchpy/test_async/test_connection.py
index 1b8cdc58..0cda23d1 100644
--- a/test_opensearchpy/test_async/test_connection.py
+++ b/test_opensearchpy/test_async/test_connection.py
@@ -43,6 +43,7 @@ from opensearchpy import AIOHttpConnection, AsyncOpenSearch, __versionstr__, ser
 from opensearchpy.compat import reraise_exceptions
 from opensearchpy.connection import Connection, async_connections
 from opensearchpy.exceptions import ConnectionError, TransportError
+from test_opensearchpy.TestHttpServer import TestHTTPServer
 
 pytestmark = pytest.mark.asyncio
 
@@ -319,72 +320,80 @@ class TestAIOHttpConnection:
             await con.close()
 
 
-class TestConnectionHttpbin:
+class TestConnectionHttpServer:
     """Tests the HTTP connection implementations against a live server E2E"""
 
-    async def httpbin_anything(self, conn, **kwargs):
-        status, headers, data = await conn.perform_request("GET", "/anything", **kwargs)
+    @classmethod
+    def setup_class(cls):
+        # Start server
+        cls.server = TestHTTPServer(port=8081)
+        cls.server.start()
+
+    @classmethod
+    def teardown_class(cls):
+        # Stop server
+        cls.server.stop()
+
+    async def httpserver(self, conn, **kwargs):
+        status, headers, data = await conn.perform_request("GET", "/", **kwargs)
         data = json.loads(data)
-        data["headers"].pop(
-            "X-Amzn-Trace-Id", None
-        )  # Remove this header as it's put there by AWS.
         return (status, data)
 
     async def test_aiohttp_connection(self):
         # Defaults
-        conn = AIOHttpConnection("httpbin.org", port=443, use_ssl=True)
+        conn = AIOHttpConnection("localhost", port=8081, use_ssl=False)
         user_agent = conn._get_default_user_agent()
-        status, data = await self.httpbin_anything(conn)
+        status, data = await self.httpserver(conn)
         assert status == 200
         assert data["method"] == "GET"
         assert data["headers"] == {
             "Content-Type": "application/json",
-            "Host": "httpbin.org",
+            "Host": "localhost:8081",
             "User-Agent": user_agent,
         }
 
         # http_compress=False
         conn = AIOHttpConnection(
-            "httpbin.org", port=443, use_ssl=True, http_compress=False
+            "localhost", port=8081, use_ssl=False, http_compress=False
         )
-        status, data = await self.httpbin_anything(conn)
+        status, data = await self.httpserver(conn)
         assert status == 200
         assert data["method"] == "GET"
         assert data["headers"] == {
             "Content-Type": "application/json",
-            "Host": "httpbin.org",
+            "Host": "localhost:8081",
             "User-Agent": user_agent,
         }
 
         # http_compress=True
         conn = AIOHttpConnection(
-            "httpbin.org", port=443, use_ssl=True, http_compress=True
+            "localhost", port=8081, use_ssl=False, http_compress=True
         )
-        status, data = await self.httpbin_anything(conn)
+        status, data = await self.httpserver(conn)
         assert status == 200
         assert data["headers"] == {
             "Accept-Encoding": "gzip,deflate",
             "Content-Type": "application/json",
-            "Host": "httpbin.org",
+            "Host": "localhost:8081",
             "User-Agent": user_agent,
         }
 
         # Headers
         conn = AIOHttpConnection(
-            "httpbin.org",
-            port=443,
-            use_ssl=True,
+            "localhost",
+            port=8081,
+            use_ssl=False,
             http_compress=True,
             headers={"header1": "value1"},
         )
-        status, data = await self.httpbin_anything(
+        status, data = await self.httpserver(
             conn, headers={"header2": "value2", "header1": "override!"}
         )
         assert status == 200
         assert data["headers"] == {
             "Accept-Encoding": "gzip,deflate",
             "Content-Type": "application/json",
-            "Host": "httpbin.org",
+            "Host": "localhost:8081",
             "Header1": "override!",
             "Header2": "value2",
             "User-Agent": user_agent,
diff --git a/test_opensearchpy/test_client/test_utils.py b/test_opensearchpy/test_client/test_utils.py
index 2b5e5a39..1a4b6809 100644
--- a/test_opensearchpy/test_client/test_utils.py
+++ b/test_opensearchpy/test_client/test_utils.py
@@ -85,6 +85,31 @@ class TestQueryParams(TestCase):
         self.func_to_wrap(headers={"X": "y"})
         self.assertEqual(self.calls[-1], ((), {"params": {}, "headers": {"x": "y"}}))
 
+    def test_non_escaping_params(self):
+        # the query_params decorator doesn't validate "timeout" it simply avoids escaping as it did
+        self.func_to_wrap(simple_param="x", timeout="4s")
+        self.assertEqual(
+            self.calls[-1],
+            ((), {"params": {"simple_param": b"x", "timeout": "4s"}, "headers": {}}),
+        )
+
+        self.func_to_wrap(simple_param="x", timeout=4, ignore=5, request_timeout=6)
+        self.assertEqual(
+            self.calls[-1],
+            (
+                (),
+                {
+                    "params": {
+                        "simple_param": b"x",
+                        "timeout": 4,
+                        "ignore": 5,
+                        "request_timeout": 6,
+                    },
+                    "headers": {},
+                },
+            ),
+        )
+
     def test_per_call_authentication(self):
         self.func_to_wrap(api_key=("name", "key"))
         self.assertEqual(
diff --git a/test_opensearchpy/test_connection.py b/test_opensearchpy/test_connection.py
index 3ed1cf0b..8d501ffe 100644
--- a/test_opensearchpy/test_connection.py
+++ b/test_opensearchpy/test_connection.py
@@ -72,6 +72,9 @@ from pytest import raises
 from opensearchpy import OpenSearch, serializer
 from opensearchpy.connection import connections
 
+if sys.version_info > (3, 0):
+    from test_opensearchpy.TestHttpServer import TestHTTPServer
+
 
 def gzip_decompress(data):
     buf = gzip.GzipFile(fileobj=io.BytesIO(data), mode="rb")
@@ -857,76 +860,87 @@ class TestRequestsConnection(TestCase):
         assert str(e.value) == "Wasn't modified!"
 
 
-class TestConnectionHttpbin:
+@pytest.mark.skipif(
+    sys.version_info < (3, 0),
+    reason="http_server is only available from python 3.x",
+)
+class TestConnectionHttpServer:
     """Tests the HTTP connection implementations against a live server E2E"""
 
-    def httpbin_anything(self, conn, **kwargs):
-        status, headers, data = conn.perform_request("GET", "/anything", **kwargs)
+    @classmethod
+    def setup_class(cls):
+        # Start server
+        cls.server = TestHTTPServer(port=8080)
+        cls.server.start()
+
+    @classmethod
+    def teardown_class(cls):
+        # Stop server
+        cls.server.stop()
+
+    def httpserver(self, conn, **kwargs):
+        status, headers, data = conn.perform_request("GET", "/", **kwargs)
         data = json.loads(data)
-        data["headers"].pop(
-            "X-Amzn-Trace-Id", None
-        )  # Remove this header as it's put there by AWS.
         return (status, data)
 
     def test_urllib3_connection(self):
         # Defaults
-        # httpbin.org can be slow sometimes. Hence the timeout
-        conn = Urllib3HttpConnection("httpbin.org", port=443, use_ssl=True, timeout=60)
+        conn = Urllib3HttpConnection("localhost", port=8080, use_ssl=False, timeout=60)
         user_agent = conn._get_default_user_agent()
-        status, data = self.httpbin_anything(conn)
+        status, data = self.httpserver(conn)
         assert status == 200
         assert data["method"] == "GET"
         assert data["headers"] == {
             "Accept-Encoding": "identity",
             "Content-Type": "application/json",
-            "Host": "httpbin.org",
+            "Host": "localhost:8080",
             "User-Agent": user_agent,
         }
 
         # http_compress=False
         conn = Urllib3HttpConnection(
-            "httpbin.org", port=443, use_ssl=True, http_compress=False, timeout=60
+            "localhost", port=8080, use_ssl=False, http_compress=False, timeout=60
         )
-        status, data = self.httpbin_anything(conn)
+        status, data = self.httpserver(conn)
         assert status == 200
         assert data["method"] == "GET"
         assert data["headers"] == {
             "Accept-Encoding": "identity",
             "Content-Type": "application/json",
-            "Host": "httpbin.org",
+            "Host": "localhost:8080",
             "User-Agent": user_agent,
         }
 
         # http_compress=True
         conn = Urllib3HttpConnection(
-            "httpbin.org", port=443, use_ssl=True, http_compress=True, timeout=60
+            "localhost", port=8080, use_ssl=False, http_compress=True, timeout=60
         )
-        status, data = self.httpbin_anything(conn)
+        status, data = self.httpserver(conn)
         assert status == 200
         assert data["headers"] == {
             "Accept-Encoding": "gzip,deflate",
             "Content-Type": "application/json",
-            "Host": "httpbin.org",
+            "Host": "localhost:8080",
             "User-Agent": user_agent,
         }
 
         # Headers
         conn = Urllib3HttpConnection(
-            "httpbin.org",
-            port=443,
-            use_ssl=True,
+            "localhost",
+            port=8080,
+            use_ssl=False,
             http_compress=True,
             headers={"header1": "value1"},
             timeout=60,
         )
-        status, data = self.httpbin_anything(
+        status, data = self.httpserver(
             conn, headers={"header2": "value2", "header1": "override!"}
         )
         assert status == 200
         assert data["headers"] == {
             "Accept-Encoding": "gzip,deflate",
             "Content-Type": "application/json",
-            "Host": "httpbin.org",
+            "Host": "localhost:8080",
             "Header1": "override!",
             "Header2": "value2",
             "User-Agent": user_agent,
@@ -939,62 +953,62 @@ class TestConnectionHttpbin:
 
     def test_requests_connection(self):
         # Defaults
-        conn = RequestsHttpConnection("httpbin.org", port=443, use_ssl=True, timeout=60)
+        conn = RequestsHttpConnection("localhost", port=8080, use_ssl=False, timeout=60)
         user_agent = conn._get_default_user_agent()
-        status, data = self.httpbin_anything(conn)
+        status, data = self.httpserver(conn)
         assert status == 200
         assert data["method"] == "GET"
         assert data["headers"] == {
             "Accept-Encoding": "identity",
             "Content-Type": "application/json",
-            "Host": "httpbin.org",
+            "Host": "localhost:8080",
             "User-Agent": user_agent,
         }
 
         # http_compress=False
         conn = RequestsHttpConnection(
-            "httpbin.org", port=443, use_ssl=True, http_compress=False, timeout=60
+            "localhost", port=8080, use_ssl=False, http_compress=False, timeout=60
         )
-        status, data = self.httpbin_anything(conn)
+        status, data = self.httpserver(conn)
         assert status == 200
         assert data["method"] == "GET"
         assert data["headers"] == {
             "Accept-Encoding": "identity",
             "Content-Type": "application/json",
-            "Host": "httpbin.org",
+            "Host": "localhost:8080",
             "User-Agent": user_agent,
         }
 
         # http_compress=True
         conn = RequestsHttpConnection(
-            "httpbin.org", port=443, use_ssl=True, http_compress=True, timeout=60
+            "localhost", port=8080, use_ssl=False, http_compress=True, timeout=60
         )
-        status, data = self.httpbin_anything(conn)
+        status, data = self.httpserver(conn)
         assert status == 200
         assert data["headers"] == {
             "Accept-Encoding": "gzip,deflate",
             "Content-Type": "application/json",
-            "Host": "httpbin.org",
+            "Host": "localhost:8080",
             "User-Agent": user_agent,
         }
 
         # Headers
         conn = RequestsHttpConnection(
-            "httpbin.org",
-            port=443,
-            use_ssl=True,
+            "localhost",
+            port=8080,
+            use_ssl=False,
             http_compress=True,
             headers={"header1": "value1"},
             timeout=60,
         )
-        status, data = self.httpbin_anything(
+        status, data = self.httpserver(
             conn, headers={"header2": "value2", "header1": "override!"}
         )
         assert status == 200
         assert data["headers"] == {
             "Accept-Encoding": "gzip,deflate",
             "Content-Type": "application/json",
-            "Host": "httpbin.org",
+            "Host": "localhost:8080",
             "Header1": "override!",
             "Header2": "value2",
             "User-Agent": user_agent,