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

feat: Errors consumer polls producer #52

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

feat: Errors consumer polls producer #52

wants to merge 29 commits into from

Conversation

lynnagara
Copy link
Member

Also adds the ProducerError enum and raises MessageRejected
if the producer queue is full.

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.

Just a clarification about the plan for the producer otherwise it is ok

Comment on lines +59 to +61
if let Err(ProducerError::QueueFull) = res {
return Err(ProcessingError::MessageRejected);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you plan to start using futures instead? The issue with this approach, I think, is that it puts you in a CPU tight loop while you wait for the message to be produced, while you can leave the CPU free by waiting on a fuiture.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think using futures might make more sense. But this is not actually a synchronous function currently and we are not waiting. It fires a delivery callback when it's done.

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.

2 participants