-
Notifications
You must be signed in to change notification settings - Fork 172
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
feat: add pause consumer #1234
base: main
Are you sure you want to change the base?
feat: add pause consumer #1234
Conversation
62e2703
to
e054054
Compare
Thanks for the PR! However, this feature has to be implemented in async-nats. Let me know if I can help you with that in any way, or reach me on NATS slack, if you want to continue the work on this of course :). |
cda3184
to
931536f
Compare
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Am I going in the right direction now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution!
This looks nice. Some comments added.
@@ -850,6 +850,64 @@ impl Stream { | |||
} | |||
} | |||
|
|||
/// Pause a [Consumer] until the given time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a note about what it means, briefly.
|
||
async fn request_pause_consumer(&self, name: &str, pause_until: Option<OffsetDateTime>) -> Result<PauseResponse, ConsumerError> { | ||
let subject = format!("CONSUMER.PAUSE.{}.{}", self.info.config.name, name); | ||
let payload = &PauseConsumerRequest{ pause_until }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use literal json!
as the struct is used only once.
@@ -3475,6 +3475,7 @@ mod jetstream { | |||
max_ack_pending: 150, | |||
}), | |||
first_sequence: Some(505), | |||
pause_until: None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add test for pause/unpause.
I'm happy to help with that.
@yordis hey! Are you interested in finishing the PR? 🙂 |
Closes #1218