Skip to content

Commit

Permalink
Fix type error when timeout is provided in connection string
Browse files Browse the repository at this point in the history
  • Loading branch information
piby180 committed Jul 13, 2024
1 parent 1ed4a28 commit faad0d7
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 4 deletions.
4 changes: 2 additions & 2 deletions pinotdb/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def cursor(self):
if not self.session or self.session.is_closed:
self.session = httpx.Client(
verify=self._kwargs.get('verify_ssl'),
timeout=self._kwargs.get('timeout'))
timeout=int(self._kwargs.get('timeout')) if self._kwargs.get('timeout') else None)

self._kwargs['session'] = self.session
cursor = Cursor(*self._args, **self._kwargs)
Expand Down Expand Up @@ -213,7 +213,7 @@ def cursor(self):
if not self.session or self.session.is_closed:
self.session = httpx.AsyncClient(
verify=self._kwargs.get('verify_ssl'),
timeout=self._kwargs.get('timeout'))
timeout=int(self._kwargs.get('timeout')) if self._kwargs.get('timeout') else None)

self._kwargs['session'] = self.session
cursor = AsyncCursor(*self._args, **self._kwargs)
Expand Down
4 changes: 2 additions & 2 deletions pinotdb/sqlalchemy.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def update_from_kwargs(self, givenkw):
kwargs["database"] = self._database = kwargs.pop("database")
kwargs["debug"] = self._debug = bool(kwargs.get("debug", False))
kwargs["verify_ssl"] = self._verify_ssl = (str(kwargs.get("verify_ssl", "true")).lower() in ['true'])
kwargs["timeout"] = self._timeout = kwargs.get("timeout", None)
kwargs["timeout"] = self._timeout = int(kwargs.get('timeout')) if kwargs.get('timeout') else None
logger.info(
"Updated pinot dialect args from %s: %s and %s",
dict(map(lambda kv: (kv[0], mask_value(kv[0], kv[1], ['password'])), kwargs.items())),
Expand All @@ -209,7 +209,7 @@ def create_connect_args(self, url):
"username": url.username,
"password": url.password,
"verify_ssl": self._verify_ssl or True,
"timeout": self._timeout or 10.0,
"timeout": int(self._timeout) if self._timeout else 10.0,
}
if self.engine_type == "multi_stage":
kwargs.update({"use_multistage_engine": True})
Expand Down
11 changes: 11 additions & 0 deletions tests/unit/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,17 @@ def test_verifies_httpx_session(self):
connection.verify_session()
# All good, no errors.

def test_verifies_timeout_kwargs(self):
connection = db.Connection(host='localhost', timeout=100)
cursor = connection.cursor()

self.assertDictEqual(
connection.session.timeout.as_dict(),
{'connect': 100.0, 'read': 100.0, 'write': 100.0, 'pool': 100.0}
)



def test_fails_to_verify_session_if_unexpected_type(self):
connection = db.Connection()
connection.session = object()
Expand Down
20 changes: 20 additions & 0 deletions tests/unit/test_sqlalchemy.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,26 @@ def test_creates_connection_args(self):
'timeout': 10.0,
})

def test_checks_timeout_in_query_params(self):
url = make_url(
'pinot://localhost:8000/query/sql?controller='
'http://localhost:9000/&&verify_ssl=True&&timeout=100')

cargs, cparams = self.dialect.create_connect_args(url)

self.assertEqual(cargs, [])
self.assertEqual(cparams, {
'debug': False,
'scheme': 'http',
'host': 'localhost',
'port': 8000,
'path': 'query/sql',
'username': None,
'password': None,
'verify_ssl': True,
'timeout': 100,
})

def test_creates_connection_args_without_query(self):
url = make_url('pinot://localhost:8000/query/sql')

Expand Down

0 comments on commit faad0d7

Please sign in to comment.