Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Producers and Consumers should accept Topics as arguments #336

Open
krisajenkins opened this issue Apr 19, 2024 · 2 comments
Open

Producers and Consumers should accept Topics as arguments #336

krisajenkins opened this issue Apr 19, 2024 · 2 comments

Comments

@krisajenkins
Copy link

I find myself writing a lot of code that looks like this:

    weather_topic = app.topic("weather_data")
    producer.produce(topic=weather_topic.name, ...)

...and the ergonomics of that aren't great. I know there's a good reason to create Topic objects, but to a beginner it's going to feel like they're wrapping up strings just to unwrap them again.

It's a small thing, but I think it would feel nicer to program if we changed the produce and consume APIs to accept either strings or Topics:

# In Producer:
def produce(topic: Union[str, Topic], ...)

# In Consumer:
def subscribe(topics: List[Union[str, Topic]], ...)

WDTY?

@stereosky
Copy link
Collaborator

I agree with this suggestion. Introducing a Topic class concept and discarding it when you use it in the Producer/Consumer can be confusing for beginners. Supporting string or topic would make it work as expected

@daniil-quix
Copy link
Collaborator

Hi @shrutimantri ,
Thanks a lot for taking a stab at this issue. It is much appreciated!

We plan to take a slightly different direction with Producers, Consumers, and Topics.

The overall idea is to keep the interfaces of kafka.Producer and kafka.Consumer as close as possible to the underlying confluent_kafka classes.
Instead, we want to introduce separate SerializingProducer and DeserializingConsumer classes that will accept Topic objects and use them also to serialize & deserialize data.

But to do that, we first need to refactor certain things in the library, so the path is not completely clear yet :)

I will close the #347 PR for now, but keep the issue open.

Cheers!

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 a pull request may close this issue.

3 participants