-
Notifications
You must be signed in to change notification settings - Fork 175
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
Use Self type in _BaseKafkaQueueConsumerFactory.new() #894
Conversation
f19e642
to
baea709
Compare
baea709
to
b11bc8b
Compare
@@ -38,6 +38,7 @@ requests = ">=2.21.0,<3.0" | |||
sentry-sdk = { version = ">=1.35.0,<2.0", optional = true } | |||
sqlalchemy = { version = ">=1.4.49,<2", optional = true } | |||
thrift-unofficial = ">=0.19.0,<1.0" | |||
typing-extensions = "^4.11.0" |
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.
This is already a dependency, just an indirect one. Moved to direct since I import from it.
@@ -19,6 +19,7 @@ | |||
from prometheus_client import Counter | |||
from prometheus_client import Gauge | |||
from prometheus_client import Histogram | |||
from typing_extensions import Self |
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.
Do you think it's worth doing
try:
from typing import Self # Python >= 3.11
except ImportError:
from typing_extensions import Self
and then doing this in pyproject.toml
:
typing-extensions = {version = "^4.11.0", python = "<3.11"}
?
I thought about that. I wound up here given the dependency still exists in
3.12, and that mypy deprecates typing_extensions when appropriate.
I think theyre both fine, this looked nicer to me. Easier to clean up.
…On Mon, Apr 29, 2024 at 16:04 Chris Kuehl ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In baseplate/frameworks/queue_consumer/kafka.py
<https://protect.checkpoint.com/v2/___https://github.com/reddit/baseplate.py/pull/894%23discussion_r1583768044___.YzJ1OnJlZGRpdDpjOmc6OTBlNTViOWI1MmZlN2E3NDA0NWNiOTE0NDE1NDY0MDM6NjpmNWYzOjIzM2ZmNTAyYmZjNzBlZGZhM2U3M2IxZDM2NjcwZGQxMDc5OTczZDgzZjVkNTFlYThiNzg0MWIxNTIyNjJkYjI6aDpU>
:
> @@ -19,6 +19,7 @@
from prometheus_client import Counter
from prometheus_client import Gauge
from prometheus_client import Histogram
+from typing_extensions import Self
Do you think it's worth doing
try:
from typing import Self # Python >= 3.11except ImportError:
from typing_extensions import Self
and then doing this in pyproject.toml:
typing-extensions = {version = "^4.11.0", python = "<3.11"}
?
—
Reply to this email directly, view it on GitHub
<https://protect.checkpoint.com/v2/___https://github.com/reddit/baseplate.py/pull/894%23pullrequestreview-2029637551___.YzJ1OnJlZGRpdDpjOmc6OTBlNTViOWI1MmZlN2E3NDA0NWNiOTE0NDE1NDY0MDM6NjphODZmOjU5N2M4ODI3YmM0NjE2YWZjOTJjZDlmMjYwMWFmNmMwZDczNDRiNzJkY2QyZjU1MmNlMTE1NGU3OTlkNzU4Y2M6aDpU>,
or unsubscribe
<https://protect.checkpoint.com/v2/___https://github.com/notifications/unsubscribe-auth/BAA37MISXR4OGOUDANLVGNDY72YWZAVCNFSM6AAAAABG63ZYCSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMRZGYZTONJVGE___.YzJ1OnJlZGRpdDpjOmc6OTBlNTViOWI1MmZlN2E3NDA0NWNiOTE0NDE1NDY0MDM6Njo5OTVhOjBhZTRkZjRhYzk1MTVlN2E2ZmNjODk1YWY0OGZhODc2ZWFkZGRhYWJiMmQ3N2IyOWY1ZTE0ZDllNzViYTQzYzE6aDpU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Closes #797
💸 TL;DR
Correct a typing annotation
🧪 Testing Steps / Validation
Lint pass in ci
✅ Checks
Before:
After: