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

Support DLQ redrive in redis #104

Merged
merged 4 commits into from
Sep 9, 2024
Merged

Conversation

jaymell
Copy link
Contributor

@jaymell jaymell commented Sep 5, 2024

Support redriving the DLQ directly from the QueueProducer. Is
this a good idea? Sure! Only supported in Redis for now.

@jaymell jaymell force-pushed the james/redrive-dlq branch 2 times, most recently from 1336068 to eb904d1 Compare September 6, 2024 14:04
@jaymell jaymell changed the title James/redrive dlq Support DLQ redrive in redis Sep 6, 2024
@jaymell jaymell force-pushed the james/redrive-dlq branch 2 times, most recently from b631524 to 8d6e8bb Compare September 6, 2024 17:30
Support redriving the DLQ directly from the QueueProducer. Is
this a good idea? Sure! Only supported in Redis for now.
1. Add missing tests to redis_cluster module
2. redis::test_backward_compatible had logic that
   should have been in redis::test_deadletter_config,
   so relocated that as appropriate.
@jaymell jaymell marked this pull request as ready for review September 6, 2024 17:34
@jaymell jaymell requested a review from a team as a code owner September 6, 2024 17:34
svix-onelson
svix-onelson previously approved these changes Sep 7, 2024
Copy link
Contributor

@svix-onelson svix-onelson left a comment

Choose a reason for hiding this comment

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

LGTM

omniqueue/src/macros.rs Show resolved Hide resolved
omniqueue/tests/it/redis.rs Outdated Show resolved Hide resolved
I'm not sure if we need to generically guarantee this in our docs,
but it seems reasonable to validate it in our tests for now.
@jaymell
Copy link
Contributor Author

jaymell commented Sep 9, 2024

@svix-onelson Mind taking a look again?

Copy link
Contributor

@svix-onelson svix-onelson left a comment

Choose a reason for hiding this comment

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

Nailed it

@svix-james svix-james merged commit a2817f2 into svix:main Sep 9, 2024
4 checks passed
@jaymell jaymell deleted the james/redrive-dlq branch September 10, 2024 17:37
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.

4 participants