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

Merged
merged 2 commits into from
Jan 10, 2025

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.

Copy link
Member

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.

@arturdryomov arturdryomov requested a review from hashhar December 19, 2024 13:05
Copy link
Member

@hashhar hashhar left a 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.

@hashhar hashhar force-pushed the ad/types-trino-request branch from a8d19c5 to 9617a51 Compare January 10, 2025 10:42
@hashhar
Copy link
Member

hashhar commented Jan 10, 2025

reordered and reworded a bit, will merge on CI. Thanks.

@hashhar hashhar merged commit 649d48d into trinodb:master Jan 10, 2025
12 checks passed
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.

2 participants