-
Notifications
You must be signed in to change notification settings - Fork 171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add missing type annotations for the "TrinoRequest" class #505
base: master
Are you sure you want to change the base?
Conversation
@@ -33,7 +33,7 @@ | |||
from requests.auth import extract_cookies_to_jar | |||
|
|||
import trino.logging | |||
from trino.client import exceptions | |||
from trino import exceptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this change:
ImportError while importing test module 'trino-python-client/tests/unit/test_types.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../.cache/pyenv/versions/3.9.21/lib/python3.9/importlib/__init__.py:127: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/unit/test_types.py:19: in <module>
from trino import types
trino/__init__.py:12: in <module>
from . import auth
trino/auth.py:36: in <module>
from trino.client import exceptions
trino/client.py:67: in <module>
from trino.auth import Authentication
E ImportError: cannot import name 'Authentication' from partially initialized module 'trino.auth' (most likely due to a circular import) (trino-python-client/trino/auth.py)
Makes sense but not sure why trino.client
was referred for exceptions
in the first place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a mistake since the beginning - 5cd6523#diff-6db3a24c52bc43426976f516411e7de756f5ee0f8f3904ca036da682aeaa840c
Thanks for noticing and fixing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll reorder the commits and merge.
Description
Type annotations for the
client.TrinoRequest
class based on usages and tests.Release notes
Release notes are required, with the following suggested text: