-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Added support for NOACK in the StreamReadGroup methods. #1154
Conversation
Adding an optional parameter ( The question then becomes:
I think 2 would probably be the safest option, but I'm open to other options. |
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.
needs to be implemented as an overload, not a breaking (non-binary-compatible) change to the existing signature
Thanks Marc, I will update the change to be an overload. It looks like I need to make the same change for #1141 as well. |
Yes,same thing there. Comment added. |
Note on making this change: the usual pattern when there are existing optional parameters is to make the old version non-optional throughout, i.e. from: Whatever SomeMethod(int a, string b, float? c = null, SomeEnum d = SomeEnum.None)
{...} to Whatever SomeMethod(int a, string b, float? c, SomeEnum d)
=> SomeMethod(a, b, c, blah, d);
Whatever SomeMethod(int a, string b, float? c = null, NewArgHere yay = blah, SomeEnum d = SomeEnum.None)
{...} This minimizes problems due to ambiguous method resolution, while ensuring that old code a: compiles, and b: continues to execute without compiling. Additionally, newly compiled code will almost always pick the new method, avoiding a stack-frame (which may or may not be inlined by the JIT). |
Apologies for the delay in getting this done...summer vacation, work, etc... I will take care of the overload for the MKSTREAM option as well. Thanks Marc! |
d18f9d7
to
2eae1c9
Compare
Merging, thanks |
The StreamReadGroup methods were updated to include a
noAck
parameter that defaults tofalse
. Whentrue
, the NOACK option is sent with the XREADGROUP command. According to the documentation, this is the equivalent of acknowledging the message when it is read and the message will not be added to the pending message list.Tests were also added to verify that messages are not added to the pending message list when this option is used.