Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

arturdryomov
Copy link
Member

Description

Type annotations for the client.TrinoRequest class based on usages and tests.

Release notes

Release notes are required, with the following suggested text:

* Add type annotations for `client.TrinoRequest`.

@cla-bot cla-bot bot added the cla-signed label Dec 19, 2024
@@ -33,7 +33,7 @@
from requests.auth import extract_cookies_to_jar

import trino.logging
from trino.client import exceptions
from trino import exceptions
Copy link
Member Author

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.

@arturdryomov arturdryomov requested a review from hashhar December 19, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

1 participant