confluentinc / confluent-kafka-python

Confluent's Kafka Python Client
http://docs.confluent.io/current/clients/confluent-kafka-python
Other
140 stars 898 forks source link

Add typing support #1408

Open palfrey opened 2 years ago

palfrey commented 2 years ago

Description

Currently, if you run mypy with a project with this in, you get error: Skipping analyzing "confluent_kafka.admin": module is installed, but missing library stubs or py.typed marker [import] or similar. https://mypy.readthedocs.io/en/stable/installed_packages.html has a guide on the ways to fix this.

How to reproduce

mypy <some code that uses confluent_kafka>

Checklist

Please provide the following information:

mhowlett commented 2 years ago

thanks for the suggestion @palfrey - yep, i would love to get more "polish the cover of the book" type things done.

prioritization coming down from product is all internal atm.

LiamClarkeNZ commented 2 years ago

Very much a +1 on this.

@jliunyu previously stated in #1268 that this is low on the agenda for Confluent as it doesn't offer value to Python users on < 3.5, but the fact that confluent-kafka-python is backed by Confluent is why we used it instead of kafka-python, as we're signed up onto Confluent Cloud with its associated support, so why not use a supported client too? (Plus some other features kafka-python is catching up on, admittedly, but the support was the big seller for CKP).

But it's not super-great when your IDE thinks that kafka_message.value() is missing a keyword argument of payload based on type hints it tried to infer from doc-strings, as it introduces a lot of false positive warnings into code using confluent-kafka-python.

I've worked around it a small amount with mypy's stubgen and manual editing of .pyi files based on API documentation, but when the API documentation (for a recent example) doesn't describe the type parameters of the on_assign callback to subscribe then it gets slow and painful to get the type hints correct.

So please show your product owner this comment if it helps any, @mhowlett :D

Other tickets that relate to a lack of type stubs:

1268 - closed and wishlisted

1310 - open and waiting

466 - open and elderly

palfrey commented 1 year ago

https://github.com/confluentinc/confluent-kafka-python/pull/1493 is an initial shot at this

Kyle-sandeman-mrdfood commented 1 year ago

Hi, just to add a comment about message.value() in PyCharm, it seems that in the docstring value (payload) is interpreted as "function signature docs". My suggestion would be value (the payload) (cannot verify whether this works right now) but it feels a bit hacky. My guess is that PyCharm looks for a valid variable name (or comma-separated ones) inside brackets. The "proper" solution would be creating a cimpl.pyi file with the correct types, although this is a maintenance headache going forward. Not sure about backwards compatibility for < 3.4 or 2.x but as palfrey says in the PR, this may not be required.

olejorgenb commented 9 months ago

From #1268 (jan. 2022)

But since the type hints are supporting from python 3.5, it's not compatible with python version lower than 3.5, we still have a lot of customers using python version lower than 3.5, it's hard for us to import this feature for now. But we may consider this feature later.

Time to reconsider in 2024?

LiamClarkeNZ commented 9 months ago

From #1268 (jan. 2022)

But since the type hints are supporting from python 3.5, it's not compatible with python version lower than 3.5, we still have a lot of customers using python version lower than 3.5, it's hard for us to import this feature for now. But we may consider this feature later.

Time to reconsider in 2024?

Especially as type hints can be provided via Typeshed, they don't need to be in the code.

Seriously though, the fact that this is low priority for Confluent makes me sad, because it's not low priority for the people using it (I'm a paying customer, I suggested we use CKP for the support, whoops!) especially when they're pushing hard on Flink which has a strong Python component.

james-johnston-thumbtack commented 9 months ago

+1 for adding type hints. Typeshed is a good alternative, but at this point, I'd also think they can surely be directly included in the main package? I find the latter works best with navigating the code with IntelliJ when Command+clicking into the functions.

I see that Python 3.7 is the oldest version being tested at this point: https://github.com/confluentinc/confluent-kafka-python/blob/5a87879681d28375a55203c4839338b13b668046/tox.ini#L2

On the contrary I'm not seeing testing for Python 3.11 and 3.12, so that seems like it would be good to add to CI/CD.

I'm a paying customer, I suggested we use CKP for the support, whoops

We're also paying Confluent Cloud customers, and we'd prefer support for features in newer Python versions, over compatibility with older Python versions, especially if the latter means forgoing things like type hints.

Python 3.4 reached end-of-life 5 years ago, I think it's time to say good-bye.

Kyle-sandeman-mrdfood commented 9 months ago

Especially as type hints can be provided via Typeshed, they don't need to be in the code.

Type hints can also be defined completely separately from runtime code in .pyi (stub/interface/header) files. Typeshed is a community effort for providing types to projects that don't do it

LiamClarkeNZ commented 9 months ago

I was trying to give an example of how separate they could be if you really needed them to be, not imply it's the only way to provide them :)

On Fri, 2 Feb 2024, 10:03 pm Kyle Sandeman, @.***> wrote:

Especially as type hints can be provided via Typeshed, they don't need to be in the code.

Type hints can also be defined completely separately from runtime code in .pyi (stub/interface/header) files. Typeshed is a community effort for providing types to projects that don't do it

— Reply to this email directly, view it on GitHub https://github.com/confluentinc/confluent-kafka-python/issues/1408#issuecomment-1923368829, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2Q2JG7GBL4Y4FCRAHYVWLYRSTXZAVCNFSM56RTHZN2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOJSGMZTMOBYGI4Q . You are receiving this because you commented.Message ID: @.***>

ykun92 commented 6 months ago

Seems someone already made this https://github.com/benbenbang/types-confluent-kafka

odimko commented 3 months ago

any updates on this? maybe @mhowlett

odimko commented 1 month ago

hey there! just checking in if there's any activity on this issue?

cc @mhowlett