aiokafka icon indicating copy to clipboard operation
aiokafka copied to clipboard

Added type annotation to public classes.

Open jackgene opened this issue 1 year ago • 0 comments

Changes

Adds type annotation to public classes, and py.typed so that client code can type check against this library.

Fixes https://github.com/aio-libs/aiokafka/issues/980 (kind of)

Note that the issue in question seems to be more ambitious in that it aims to provide type information to the entire library:

  1. Allowing bugs to be discovered
  2. Allowing developers consuming this library to have better type information to work with.

This PR only aims to solve the latter.

Per providing type annotations, there are several ways to distribute type information:

  • inline type annotations (preferred)
  • type stub files included in the package
  • a separate companion type stub package
  • type stubs in the typeshed repository

This is the first option, and makes changes to .py files mostly just adding type annotations. If you prefer to go with the second option (which doesn't touch any .py file), please have a look at https://github.com/aio-libs/aiokafka/pull/1075 instead.

Details about this PR:

  • Using Python 3.10+ typing, which is available in 3.9 via from __future__ import annotations
    • x | None instead of Optional[x]
    • x | y instead of Union[x, y]
    • list[x]/set[x]/dict[x, y] instead of typing.List[x]/typing.Set[x]/typing.Dict[x, y]
  • For the most part, all I'm doing is explicitly annotate methods as documented in docstring
  • There are special cases though where the docstring had to be updated, e.g.,:
    • docstring says list, but varargs/default value is tuple
    • docstring doesn't say optional, but default value is None

Note that this is introducing a breaking change that may affect a small minority of users. The key/value serializer/deserializer functions no longer optional (with the identity function defined as the default parameter). This is so that the generic parameters KT/VT can be inferred.

This means that:

  • The (de)serialization behavior is identical at runtime
  • As long as clients do not explicitly set None as their key/value serializer/deserializer function, there should be no change.

However, if a client is explicitly passing in None, e.g., with AIOKafkaConsumer:

# OK, most people probably do this anyway, this is from the README
consumer = AIOKafkaConsumer(
    'my_topic', 'my_other_topic',
    bootstrap_servers='localhost:9092',
    group_id='my-group'
)

# Not OK, key_deserializer/value_deserializer cannot be None
consumer = AIOKafkaConsumer(
    'my_topic', 'my_other_topic',
    bootstrap_servers='localhost:9092',
    group_id='my-group',
    key_deserializer=None,
    value_deserializer=None
)

Or with AIOKafkaProducer:

# OK, most people probably do this anyway, this is from the README
AIOKafkaProducer(
    bootstrap_servers='localhost:9092'
)

# Not OK, key_deserializer/value_deserializer cannot be None
AIOKafkaProducer(
    bootstrap_servers='localhost:9092'
    key_serializer=None,
    value_serializer=None
)

The type checker will reject it. image

You can ignore the type checker though, and the application should work the same as before, as this PR does not change any runtime logic.

Checklist

  • [ ] I think the code is well written
  • [ ] Unit tests for the changes exist
  • [ ] Documentation reflects the changes
  • [ ] Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> (e.g. 588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the PR
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: Fix issue with non-ascii contents in doctest text files.

jackgene avatar Nov 15 '24 23:11 jackgene