add type annotations to kombu/transport/azurestoragequeues #1632
Conversation
jasonwbarnett
force-pushed
the
annotate/azure-storage-queue
branch
4 times, most recently
from
9cbb53e
to
73bca59
Compare
| no_ack = True | ||
| _noack_queues = set() | ||
| domain_format: str = 'kombu%(vhost)s' | ||
| _queue_service: QueueServiceClient | None = None |
There was a problem hiding this comment.
Shouldn't _queue_service be Optional?
There was a problem hiding this comment.
@thedrow I agree, but the pre-commit.ci automatically corrected it and switched it from Optional[QueueServiceClient] to this. I just assumed this was an alternative way to declare the same thing, but outside of this project I would always use the Optional[QueueServiceClient] notation.
The docs you linked also mention this is an alternative syntax and that the two are equivalent.
Optional[X]is equivalent toX | None(orUnion[X, None]).
What should I do?
There was a problem hiding this comment.
I made the updates, and I assume pre-commit.ci will add a commit correcting it, but we'll see.
There was a problem hiding this comment.
Yeah, pre-commit.ci corrected it so I just re-pushed the original content that you commented on. Should I try to update the pre-commit.ci configuration or merge as is?
There was a problem hiding this comment.
If you can update the pre-commit config that would be great but I wouldn't block merging this PR without that config change.
| default_port = None | ||
| can_parse_url = True | ||
| polling_interval: int = 1 | ||
| default_port: int | None = None |
jasonwbarnett
force-pushed
the
annotate/azure-storage-queue
branch
2 times, most recently
from
864c0bf
to
73bca59
Compare
@thedrow I reviewed the inline comments and responded with why I didn't do that. Please let me know how you would like me to proceed. |
jasonwbarnett
force-pushed
the
annotate/azure-storage-queue
branch
from
73bca59
to
38fb6af
Compare
|
@thedrow I ended up finding the root cause of this. It turns out PEP 604 docs: https://peps.python.org/pep-0604/ If you'd like to keep pyupgrade from updating this, we could use the following argument; Standing by waiting for your input. |
jasonwbarnett
force-pushed
the
annotate/azure-storage-queue
branch
from
38fb6af
to
1ebb470
Compare
|
I just rebased this PR off of main. |
|
Yeah let's configure pyupdate to keep |
Per celery#1632 @thedrow asked to add this argument so `Optional` is still used.
jasonwbarnett
force-pushed
the
annotate/azure-storage-queue
branch
from
1ebb470
to
976e7eb
Compare
jasonwbarnett
force-pushed
the
annotate/azure-storage-queue
branch
from
8aa637d
to
f2c700a
Compare
jasonwbarnett
force-pushed
the
annotate/azure-storage-queue
branch
from
f82a494
to
9a04ec0
Compare
|
LGTM but I'll let @thedrow have the final word here |
This isn't complete, but at least it's a start. I'm open to any feedback.