Fix socket timeout test #144

mattdeekay · 2023-06-09T18:30:23Z

test_socket_timeout_user_defined e2e test worked for the previous version of the driver before we introduced the urllib3.connectionpool to reuse http connection. The error that is thrown is now urllib3.connnectionpool.ReadTimeoutError

susodapop · 2023-06-09T18:34:07Z

hmmm test still fails for me...it might just be my environment is scuffed.

========================================================== test session starts ==========================================================
platform darwin -- Python 3.10.5, pytest-7.3.1, pluggy-1.0.0
rootdir: /Users/jesse.whitehouse/Developer/sdks/py
collected 1 item                                                                                                                        

tests/e2e/driver_tests.py F                                                                                                       [100%]

=============================================================== FAILURES ================================================================
__________________________________________ PySQLCoreTestSuite.test_socket_timeout_user_defined __________________________________________

self = <driver_tests.PySQLCoreTestSuite testMethod=test_socket_timeout_user_defined>

    @skipIf(pysql_has_version('<', '2'), 'requires pysql v2')
    def test_socket_timeout_user_defined(self):
        #  We expect to see a TimeoutError when the socket timeout is only
        #  1 sec for a query that takes longer than that to process
>       with self.assertRaises(RequestError) as cm:
E       AssertionError: RequestError not raised

tests/e2e/driver_tests.py:476: AssertionError
======================================================== short test summary info ========================================================
FAILED tests/e2e/driver_tests.py::PySQLCoreTestSuite::test_socket_timeout_user_defined - AssertionError: RequestError not raised
=========================================================== 1 failed in 1.98s ===========================================================

susodapop · 2023-06-09T18:42:44Z

I did a fresh checkout of the repository and I see the same error as above. Can you run poetry run pip freeze so we can see what versions of each dependency you have installed? Here are mine:

alembic==1.10.4
astroid==2.11.7
black==22.12.0
certifi==2023.5.7
charset-normalizer==3.1.0
click==8.1.3
-e git+ssh://[email protected]/databricks/databricks-sql-python.git@a9d40f65ec59fe0a8aa3641ae9dc55dcd18b6abd#egg=databricks_sql_connector
dill==0.3.6
et-xmlfile==1.1.0
exceptiongroup==1.1.1
greenlet==2.0.2
idna==3.4
iniconfig==2.0.0
isort==5.11.5
lazy-object-proxy==1.9.0
lz4==4.3.2
Mako==1.2.4
MarkupSafe==2.1.2
mccabe==0.7.0
mypy==0.950
mypy-extensions==1.0.0
numpy==1.24.3
oauthlib==3.2.2
openpyxl==3.1.2
packaging==23.1
pandas==1.3.5
pathspec==0.11.1
platformdirs==3.5.0
pluggy==1.0.0
pyarrow==12.0.0
pylint==2.13.9
pytest==7.3.1
python-dateutil==2.8.2
pytz==2023.3
requests==2.30.0
six==1.16.0
SQLAlchemy==1.4.48
thrift==0.16.0
tomli==2.0.1
typing_extensions==4.5.0
urllib3==2.0.2
wrapt==1.15.0

mattdeekay · 2023-06-09T18:44:39Z

These are mine:

alembic==1.10.4
astroid==2.11.7
black==22.12.0
certifi==2023.5.7
charset-normalizer==3.1.0
click==8.1.3
-e git+https://[email protected]/mattdeekay/databricks-sql-python.git@a9d40f65ec59fe0a8aa3641ae9dc55dcd18b6abd#egg=databricks_sql_connector
dill==0.3.6
et-xmlfile==1.1.0
idna==3.4
iniconfig==2.0.0
isort==5.11.5
lazy-object-proxy==1.9.0
lz4==4.3.2
Mako==1.2.4
MarkupSafe==2.1.2
mccabe==0.7.0
mypy==0.950
mypy-extensions==1.0.0
numpy==1.24.3
oauthlib==3.2.2
openpyxl==3.1.2
packaging==23.1
pandas==1.3.5
pathspec==0.11.1
platformdirs==3.5.0
pluggy==1.0.0
pyarrow==12.0.0
pylint==2.13.9
pytest==7.3.1
python-dateutil==2.8.2
pytz==2023.3
requests==2.30.0
six==1.16.0
SQLAlchemy==1.4.48
thrift==0.16.0
types-python-dateutil==2.8.19.13
types-requests==2.31.0.1
types-six==1.16.21.8
types-urllib3==1.26.25.13
typing_extensions==4.5.0
urllib3==2.0.2
wrapt==1.15.0

mattdeekay · 2023-06-09T18:48:16Z

Seems like you have exceptiongroup==1.1.1 and greenlet==2.0.2 and I have types-python-dateutil==2.8.19.13, types-requests==2.31.0.1, types-six==1.16.21.8, types-urllib3==1.26.25.13?

susodapop · 2023-06-09T18:48:17Z

What version of Python do you use?

mattdeekay · 2023-06-09T18:50:05Z

Virtualenv
Python: 3.11.3
Implementation: CPython

System
Platform: darwin
OS: posix
Python: 3.11.3

mattdeekay · 2023-06-09T18:54:25Z

❯ poetry run python -m pytest tests/e2e/driver_tests.py::PySQLCoreTestSuite::test_socket_timeout_user_defined
========================================== test session starts ===========================================
platform darwin -- Python 3.11.3, pytest-7.3.1, pluggy-1.0.0
collected 1 item

tests/e2e/driver_tests.py .                                                                        [100%]
====================================== 1 passed, 1 warning in 1.84s ======================================

susodapop · 2023-06-09T19:02:26Z

I wonder if it's a change in Python 3.11. I'm going to try that way.

mattdeekay · 2023-06-09T19:07:25Z

I ran:

poetry env use 3.10.5
poetry install
poetry run python -m pytest tests/e2e/driver_tests.py::PySQLCoreTestSuite::test_socket_timeout_user_defined

And the test is passing for platform darwin -- Python 3.10.11, pytest-7.3.1, pluggy-1.0.0

susodapop · 2023-06-09T23:32:59Z

How strange. I tried with Python 3.11 and have the same failure as before. It doesn't raise a RequestError

=================================================================== test session starts ====================================================================
platform darwin -- Python 3.11.4, pytest-7.3.1, pluggy-1.0.0
rootdir: /Users/jesse.whitehouse/Developer/temp-delete/databricks-sql-python
collected 1 item                                                                                                                                           

tests/e2e/driver_tests.py F                                                                                                                          [100%]

========================================================================= FAILURES =========================================================================
___________________________________________________ PySQLCoreTestSuite.test_socket_timeout_user_defined ____________________________________________________

self = <driver_tests.PySQLCoreTestSuite testMethod=test_socket_timeout_user_defined>

    @skipIf(pysql_has_version('<', '2'), 'requires pysql v2')
    def test_socket_timeout_user_defined(self):
        #  We expect to see a TimeoutError when the socket timeout is only
        #  1 sec for a query that takes longer than that to process
>       with self.assertRaises(RequestError) as cm:
E       AssertionError: RequestError not raised

tests/e2e/driver_tests.py:476: AssertionError
===================================================================== warnings summary =====================================================================
tests/e2e/driver_tests.py:33
  /Users/jesse.whitehouse/Developer/temp-delete/databricks-sql-python/tests/e2e/driver_tests.py:33: DeprecationWarning: unittest.getTestCaseNames() is deprecated and will be removed in Python 3.13. Please use unittest.TestLoader.getTestCaseNames() instead.
    for name in loader.getTestCaseNames(DecimalTestsMixin, 'test_'):

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================================================================= short test summary info ==================================================================
FAILED tests/e2e/driver_tests.py::PySQLCoreTestSuite::test_socket_timeout_user_defined - AssertionError: RequestError not raised
=============================================================== 1 failed, 1 warning in 2.27s ===============================================================

Right now we're catching all HTTP errors in a weird way that causes the ReadTimeoutError to not be wrapped in a RequestError. Rather than muck around with the internals of our retry behavior and maybe break something, I'm just updating the test to reflect the way the connector actually functions. The goal of this test is just to ensure that when we set _socket_timeout that the connections are in-fact timed out. It's less important what the exception class is. Signed-off-by: Jesse Whitehouse <[email protected]>

susodapop

I rewrote this so it passes on my machine.

Signed-off-by: Jesse Whitehouse <[email protected]>

susodapop · 2023-07-12T20:48:33Z

synced with main to fix merge conflict on the changelog -- also used this opportunity to fix a doubled-entry in the CHANGELOG.

susodapop · 2023-07-12T20:49:55Z

I confirmed with @mattdeekay that this passes on their machine too.

Signed-off-by: Jesse Whitehouse <[email protected]> Co-authored-by: Jesse Whitehouse <[email protected]>

mattdeekay requested a review from susodapop as a code owner June 9, 2023 18:30

mattdeekay self-assigned this Jun 9, 2023

mattdeekay requested review from arikfr, yunbodeng-db and andrefurlan-db as code owners June 9, 2023 18:30

susodapop force-pushed the fix-socket-timeout-test branch from a9d40f6 to c2f23e6 Compare July 12, 2023 20:32

susodapop approved these changes Jul 12, 2023

View reviewed changes

Merge branch 'main' into fix-socket-timeout-test

5ae39f5

Signed-off-by: Jesse Whitehouse <[email protected]>

susodapop merged commit 1eef432 into databricks:main Jul 12, 2023

susodapop pushed a commit to unj1m/databricks-sql-python that referenced this pull request Sep 19, 2023

Fix socket timeout test (databricks#144)

a19e4c2

Signed-off-by: Jesse Whitehouse <[email protected]> Co-authored-by: Jesse Whitehouse <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix socket timeout test #144

Fix socket timeout test #144

mattdeekay commented Jun 9, 2023

susodapop commented Jun 9, 2023

susodapop commented Jun 9, 2023

mattdeekay commented Jun 9, 2023

mattdeekay commented Jun 9, 2023

susodapop commented Jun 9, 2023

mattdeekay commented Jun 9, 2023

mattdeekay commented Jun 9, 2023 •

edited

Loading

susodapop commented Jun 9, 2023

mattdeekay commented Jun 9, 2023

susodapop commented Jun 9, 2023

susodapop left a comment

susodapop commented Jul 12, 2023

susodapop commented Jul 12, 2023

Fix socket timeout test #144

Fix socket timeout test #144

Conversation

mattdeekay commented Jun 9, 2023

susodapop commented Jun 9, 2023

susodapop commented Jun 9, 2023

mattdeekay commented Jun 9, 2023

mattdeekay commented Jun 9, 2023

susodapop commented Jun 9, 2023

mattdeekay commented Jun 9, 2023

mattdeekay commented Jun 9, 2023 • edited Loading

susodapop commented Jun 9, 2023

mattdeekay commented Jun 9, 2023

susodapop commented Jun 9, 2023

susodapop left a comment

Choose a reason for hiding this comment

susodapop commented Jul 12, 2023

susodapop commented Jul 12, 2023

mattdeekay commented Jun 9, 2023 •

edited

Loading