-
Notifications
You must be signed in to change notification settings - Fork 151
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
Pause and Resume consumer #874
Conversation
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.
looks good. just a quick question below.
@@ -144,7 +144,8 @@ public static DateTime AsDate(JSONNode node) | |||
{ | |||
try | |||
{ | |||
return DateTime.Parse(node.Value).ToUniversalTime(); | |||
DateTime dt = DateTime.Parse(node.Value).ToUniversalTime(); | |||
return dt.Year == 1 ? DateTime.MinValue : dt; // b/c server sends a value that doesn't quite parse to DateTime.MinValue |
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 don't get this one :-(
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.
DateTime.MinValue.ToUniversalTime() resolves to 1/1/0001 5:00:00 AM
But for whatever reason the value the server sends 0001-01-01T00:00:00Z
when resolved with
DateTime.Parse(node.Value).ToUniversalTime()
ends up with 1/2/0001 12:00:00 AM
So I decdide that anything with year 1 must have meant min value. I can't imagine real server date data will have a date of year 1.
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 think I'm going to do it a different way and revert this. I can check if paused is false when reading the json, and if it is, then I don't even need to parse the time. This is really an issue with JSONNode
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.
LGTM
Resolves https://github.com/nats-io/nats.net/issues/867
Adds support for pausing and resuming consumers.
IJetStreamManagement
now has methods tojsm.PauseConsumer(stream, consumer, pauseUntil)
, as well asjsm.ResumeConsumer(stream, consumer)
.src/examples
.ConsumerConfiguration
/builder now has support for settingpauseUntil
as well. Allowing to set it during consumer creation.