confluent-kafka-python
confluent-kafka-python copied to clipboard
confluentinc
Typing support
Fixes https://github.com/confluentinc/confluent-kafka-python/issues/1408
This PR does at least an initial pass of adding typing support. Please note it does not support Python 2 at the moment (although that might be getting dropped?) or before 3.5. OTOH, given Python <3.7 is EoL, I don't think they should be supported, but you might want take a different approach, which would need conversion to comment-based typing. This is doable, but a chunk more work and wanted to check if that's needed.
I'd also note that my usual approach on these things is "use the existing CI support for all the version testing" but that doesn't appear to be doable here as it's not setup for that? Relatedly, my formatting is a little wonky, which usually I'd sort out with black, which appears to have been kinda setup and I'd love to get that resolved before this got merged.
~~It also won't fully work until Avro releases a new version with typing support. Note that this won't change the runtime support requirements, only the requirements if you want to do type checking.~~
Any thoughts?
I'm also seeing unrelated build failures which is always fun :)
Ah, no wait, my fault. 59f4069b0c2aaac38d8151b484f30c5318d2a915 fixes the problem by not using a pyproject.toml as that made pip install do things without the important include paths.
@palfrey Will you continue this PR? :)
Potentially. This is blocked on avro releasing a new version with the changes this needs and ideally on black getting integrated (as was being done in https://github.com/confluentinc/confluent-kafka-python/pull/1352 and then seems to be abandoned). If those get sorted, I can then go through and fix the issues here, but there's no point in repeatedly fixing all the things until then.
@palfrey apache/avro#1952 seems to be merged.
But no release (https://github.com/apache/avro/releases) since last August
@palfrey apache/avro#1952 seems to be merged.
But no release (apache/avro/releases) since last August
just FYI avro finally published a release 2 days ago!
Is this PR going to be merged soon? Is there any blocker or changes left before merging this? @palfrey Do you need to sign the Contributor License Agreement?
Is this PR going to be merged soon? Is there any blocker or changes left before merging this? @palfrey Do you need to sign the Contributor License Agreement?
Didn't know there was a CLA and TBH I wouldn't have done this if I'd known about it, but signed now. Waiting on review.
cc @jkuhnashconfluent @emasab @pranavrth for review since you three were on the last PRs to be merged into this repo.
@joaoe @Viicos are you the ones that are need to review this or is someone else?
@joaoe @Viicos are you the ones that are need to review this or is someone else?
I'm not a maintainer of the repo, so I can't do much. If you get no answers from them, the backup plan would be to create stubs but this is generally not the prefered way
Hey Guys,
Sorry for the late reply. We couldn't pay much attention to this PR.
This is an important PR which we would like to incorporate in our client. I will try to get some time in coming weeks for reviewing this PR.
Hey Guys,
Sorry for the late reply. We couldn't pay much attention to this PR.
This is an important PR which we would like to incorporate in our client. I will try to get some time in coming weeks for reviewing this PR.
How's that going?
It's a shame that all of this work has been done and the "coming weeks" has slowly turned into 4 and a half months. I fear that if this sits much longer, this contribution will start to grow stale and require additional work to get in a state that it can be merged again. I think having type hints is exceptionally valuable not just for the maintenance of this project but also the maintenance of any projects that consume this library and I would love to see merging this PR find its way back onto Confluent Inc's list of priorities.
I fear that if this sits much longer, this contribution will start to grow stale and require additional work to get in a state that it can be merged again.
I'm mostly willing to do the work to un-stale it, but only when I know there will actually be review and possible merge of it, and I'm otherwise ignoring conflicts until that point. If I'd seen a first round of review that at least looked at what was done so far (and ideally also addressed the Python versioning and black issues) then I'd come back to this, but so far I'm seeing no indications of this actually occuring any time soon despite prior comments on this PR.
@palfrey have you considered moving the effort to Typeshed?
I don't know what it entails but if I understand correctly it will actually have a chance to be merged 😔
@palfrey have you considered moving the effort to Typeshed?
I don't know what it entails but if I understand correctly it will actually have a chance to be merged 😔
I hadn't. It's a fair amount of work (ripping out the types from the individual .py files and redoing as just the signatures in .pyi files), and TBH, I'm not exactly enthused about that, especially as I don't use the library currently. If this comes back to life, I'm willing to fix things here though.
OTOH, if anyone else wants to take what I've done here and do that work in Typeshed, awesome. Some level of credit/mention would be appreciated, but otherwise, go for it!
