Skip to content

Explicitly Export PartitionMetadata to Satisfy Type Checkers#2225

Open
George Berdovskiy (george-sentry) wants to merge 1 commit intoconfluentinc:masterfrom
george-sentry:feature/export-partition-metadata
Open

Explicitly Export PartitionMetadata to Satisfy Type Checkers#2225
George Berdovskiy (george-sentry) wants to merge 1 commit intoconfluentinc:masterfrom
george-sentry:feature/export-partition-metadata

Conversation

@george-sentry
Copy link
Copy Markdown

What

At Sentry, we use this Python package to write Kafka consumers and producers. Some of our references the PartitionMetadata class, which is defined but not explicitly exported. As a result, when we attempt to import this class (and possibly others), type checking fails with an error like this one.

$ python -m mypy
src/sentry/monitors/tasks/clock_pulse.py:11: error: Module "confluent_kafka.admin" does not explicitly export attribute "PartitionMetadata"  [attr-defined]
tests/sentry/monitors/tasks/test_clock_pulse.py:6: error: Module "confluent_kafka.admin" does not explicitly export attribute "PartitionMetadata"  [attr-defined]
Found 2 errors in 2 files (checked 7783 source files)

This requires working around the type checker (specifically mypy) with # type: ignore[attr-defined], defeating the purpose of using types in the first place.

Checklist

  • Contains customer facing changes? Yes, the class PartitionMetadata is now explicitly exported.
  • Did you add sufficient unit test and/or integration test coverage for this PR? No, because runtime behavior is unaffected.

References

N/A

Test & Review

I tested these changes by pointing the Sentry monolith to this version of confluent-kafka instead of the published one. Follow these instructions to set up Sentry locally.

$ brew install librdkafka

$ export CPATH="$(brew --prefix)/include${CPATH:+:$CPATH}"
$ export LIBRARY_PATH="$(brew --prefix)/lib${LIBRARY_PATH:+:$LIBRARY_PATH}"
$ export PKG_CONFIG_PATH="$(brew --prefix)/lib/pkgconfig${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}"

$ uv pip install -e ~/Desktop/Repositories/confluent-kafka-python --python .venv/bin/python

$ python -m mypy
Success: no issues found in 7783 source files

Open Questions

  • Is there a good reason for PartitionMetadata to not be publicly exported?
  • Are there any other types that should be explicitly exported?

Copilot AI review requested due to automatic review settings March 31, 2026 19:19
@confluent-cla-assistant
Copy link
Copy Markdown

confluent-cla-assistant bot commented Mar 31, 2026

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

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes PartitionMetadata explicitly re-exported from confluent_kafka.admin so type checkers (e.g., mypy with strict re-export rules) accept from confluent_kafka.admin import PartitionMetadata without requiring # type: ignore[attr-defined].

Changes:

  • Split the _metadata imports so PartitionMetadata is explicitly re-exported via from ... import PartitionMetadata as PartitionMetadata.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 75 to +77
from ._metadata import BrokerMetadata # noqa: F401
from ._metadata import ClusterMetadata, GroupMember, GroupMetadata, PartitionMetadata, TopicMetadata # noqa: F401
from ._metadata import ClusterMetadata, GroupMember, GroupMetadata, TopicMetadata # noqa: F401
from ._metadata import PartitionMetadata as PartitionMetadata # noqa: F401
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds an explicit re-export for PartitionMetadata, but the rest of the re-exported symbols in this module are still implicit from a type-checker perspective. To avoid having to special-case additional symbols later (and to make the public surface area unambiguous), consider defining an explicit __all__ for the admin package (or consistently using from ... import X as X for all intended re-exports).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants