Set up type hinting: C extension stubs + typing for serializing_producer + deserializing_consumer #2041

fangnx · 2025-09-05T15:56:28Z

What

First PR to set up type hinting for this repo. This PR includes:

C extension stubs (cimpl.pyi). Note that stubgen didn't work well with our confluent_kafka module and only generated the stubs partially, so some manual parsing of C code (with AI) was used to fill in the gaps
Add types to core files (serializing_producer, deserialzier_consumer, etc.)
Add types to admin files, except for init.py (to avoid PR becoming too big)

Type hinting for other python files will be covered in follow-up PRs

Checklist

Contains customer facing changes? Including API/behavior changes
Did you add sufficient unit test and/or integration test coverage for this PR?
- If not, please explain why it is not required

References

JIRA: https://confluentinc.atlassian.net/browse/DGS-22076

Test & Review

Open questions / Follow-ups

confluent-cla-assistant · 2025-09-05T15:56:39Z

🎉 All Contributor License Agreements have been signed. Ready to merge.
_{Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.}

Copilot

Pull Request Overview

This PR introduces initial type hinting support for the confluent_kafka Python package by adding type stubs for the C extension and typing annotations to core serialization classes.

Establishes type safety foundation with C extension stubs and centralized type definitions
Adds comprehensive type annotations to producer and consumer serialization classes
Creates type-aware error handling hierarchy

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/confluent_kafka/cimpl.pyi	Type stubs for C extension classes and functions
src/confluent_kafka/_types.py	Centralized type aliases for the package
src/confluent_kafka/py.typed	Marker file indicating package supports typing
src/confluent_kafka/serializing_producer.py	Type annotations for SerializingProducer class
src/confluent_kafka/deserializing_consumer.py	Type annotations for DeserializingConsumer class
src/confluent_kafka/error.py	Type annotations for error classes
src/confluent_kafka/init.py	Type annotations for utility classes

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-05T20:01:19Z

src/confluent_kafka/deserializing_consumer.py


        super(DeserializingConsumer, self).__init__(conf_copy)

-    def poll(self, timeout=-1):
+    def poll(self, timeout: float = -1) -> Optional['Message']:


The return type annotation uses forward reference quotes around 'Message' but Message is already imported on line 21. Remove the quotes to use the directly imported type.

Suggested change

def poll(self, timeout: float = -1) -> Optional['Message']:

def poll(self, timeout: float = -1) -> Optional[Message]:

sonarqube-confluent · 2025-09-05T23:44:34Z

Analysis Details

1 Issue

0 Bugs
0 Vulnerabilities
1 Code Smell

Coverage and Duplications

97.40% Coverage (66.50% Estimated after merge)
No duplication information (5.50% Estimated after merge)

Project ID: confluent-kafka-python

View in SonarQube

MSeal

Would like to spend some time over zoom talking through the work on this PR but overall looks reasonable as a solution PR

MSeal · 2025-09-06T00:10:33Z

src/confluent_kafka/_model/__init__.py

@@ -60,7 +61,7 @@ class ConsumerGroupTopicPartitions:
        List of topic partitions information.
    """

-    def __init__(self, group_id, topic_partitions=None):
+    def __init__(self, group_id: str, topic_partitions: Optional[List['cimpl.TopicPartition']] = None) -> None:


Why'd this need the cimpl path prefix to work here? Curious to chat about how the cbinding type errors were worked out in development

We are only importing the module from .. import cimpl so the type checker would not find the directly referenced TopicPartition. We can switch to from ..cimpl import TopicPartition to make the code a bit cleaner

MSeal · 2025-09-06T00:12:12Z

src/confluent_kafka/_util/conversion_util.py

 from enum import Enum

+# Generic type for enum conversion
+E = TypeVar('E', bound=Enum)


Why do we need this? Can't we just use type[Enum] below?

Yes I think we can just leave it type[Enum]. I think Type[E] creates a stricter type relationship with the specific enum class, but it overcomplicates the code a bit and I don't think this function (meant to handle types itself) can benefit from it

MSeal · 2025-09-06T00:13:11Z

src/confluent_kafka/admin/_cluster.py

        self.cluster_id = cluster_id
        self.controller = controller
        self.nodes = nodes
-        self.authorized_operations = None
+        self.authorized_operations: Optional[List[AclOperation]] = None


Why the hint here and why does it differ from the hint in the def?

Fixed. Also update authorized_operations to be more specific: Optional[List[Union[str, int, AclOperation]]], as those 3 types are handled in ConversionUtil.convert_to_enum

MSeal · 2025-09-06T00:13:37Z

src/confluent_kafka/admin/_scram.py

 from enum import Enum

+from .. import cimpl


Why the move?

Honestly I think cursor moved it during the first pass and I didn't notice it. But after some investigation, I think the move makes sense as it adheres to the pep8 import rule: https://peps.python.org/pep-0008/#imports

MSeal · 2025-09-06T00:22:13Z

src/confluent_kafka/cimpl.pyi

@@ -0,0 +1,443 @@
+"""


I do dislike how we'll have two locations for type/actual definitions we need to maintain now. However it may be the ideal solution for now. This really makes me want to try a pass on converting the c files to Cython and then getting the type hinting for free while avoiding manually defining so many Python things in C binding calls. Not in scope for this PR though it would eliminate the above dual definition problem. For now let's add a warning comment to the C code files to update this file when/if editing interfaces therein

fangnx added 3 commits September 4, 2025 10:20

update

0caa2e1

remove py.typed for now

ce00394

update

1a80530

fix cimply and add types to serde producer/consumer

b20981a

fangnx marked this pull request as ready for review September 5, 2025 18:45

Copilot AI review requested due to automatic review settings September 5, 2025 18:45

fangnx requested review from MSeal and a team as code owners September 5, 2025 18:45

This comment was marked as outdated.

Sign in to view

fangnx changed the title ~~Add type hinting~~ Set up type hinting: C extension stubs + typing for serializing_producer + deserializing_consumer Sep 5, 2025

fangnx requested a review from Copilot September 5, 2025 20:00

Copilot AI reviewed Sep 5, 2025

View reviewed changes

admin

fead29d

MSeal reviewed Sep 6, 2025

View reviewed changes

fangnx added 3 commits September 8, 2025 23:18

address feedback

06f3336

add warning to stub and c files; admin typing more

378295b

add accidentally removed md files

7b378e7

fangnx requested a review from MSeal September 9, 2025 21:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Set up type hinting: C extension stubs + typing for serializing_producer + deserializing_consumer #2041

Set up type hinting: C extension stubs + typing for serializing_producer + deserializing_consumer #2041

fangnx commented Sep 5, 2025 •

edited

Loading

Uh oh!

confluent-cla-assistant bot commented Sep 5, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 5, 2025

Uh oh!

sonarqube-confluent bot commented Sep 5, 2025

Uh oh!

MSeal left a comment

Uh oh!

MSeal Sep 6, 2025

Uh oh!

fangnx Sep 8, 2025

Uh oh!

MSeal Sep 6, 2025

Uh oh!

fangnx Sep 9, 2025

Uh oh!

MSeal Sep 6, 2025

Uh oh!

fangnx Sep 9, 2025

Uh oh!

MSeal Sep 6, 2025

Uh oh!

fangnx Sep 9, 2025

Uh oh!

MSeal Sep 6, 2025

Uh oh!

Uh oh!

	def poll(self, timeout: float = -1) -> Optional['Message']:
	def poll(self, timeout: float = -1) -> Optional[Message]:

Set up type hinting: C extension stubs + typing for serializing_producer + deserializing_consumer #2041

Are you sure you want to change the base?

Set up type hinting: C extension stubs + typing for serializing_producer + deserializing_consumer #2041

Conversation

fangnx commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Checklist

References

Test & Review

Open questions / Follow-ups

Uh oh!

confluent-cla-assistant bot commented Sep 5, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

sonarqube-confluent bot commented Sep 5, 2025

Analysis Details

1 Issue

Coverage and Duplications

Uh oh!

MSeal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fangnx commented Sep 5, 2025 •

edited

Loading