Skip to content
This repository was archived by the owner on Nov 8, 2023. It is now read-only.

ref: Remove lifetime annotations from ArroyoConsumer #32

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lynnagara
Copy link
Member

This is an attempt to simplify the ArroyoConsumer lifetimes by
changing the LocalConsumer to have ownership of the LocalBroker.

Since this consumer/broker only used in place of the KafkaConsumer
for testing, I think this change is fairly harmless as it's not
required for the broker to live outside the consumer.

This is an attempt to simplify the ArroyoConsumer lifetimes by
changing the LocalConsumer to have ownership of the LocalBroker.

Since this consumer/broker only used in place of the KafkaConsumer
for testing, I think this change is fairly harmless as it's not
required for the broker to live outside the consumer.
Copy link
Collaborator

@fpacifici fpacifici left a comment

Choose a reason for hiding this comment

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

We would have a problem if the consumer owns the broker: you cannot share a broker between a producer and a consumer if you are testing that scenario.
I would not drop that scenario.

@lynnagara
Copy link
Member Author

lynnagara commented Jun 10, 2022

@fpacifici But there is no such producer in this library. Just a produce method on the broker. It seems you are implying some pretty big changes to how this works currently - that there is going to be a local producer who gets a reference to a broker.

@fpacifici
Copy link
Collaborator

https://github.com/getsentry/arroyo/blob/main/arroyo/backends/local/backend.py#L356

We have the local producer in Arroyo. Originally I did not port it just because I did not have time but that was the intention. Any reason for not doing it in the rust version ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants