-
Notifications
You must be signed in to change notification settings - Fork 64
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 custom streams filter in nack interceptor #275
Conversation
LGTM @aalekseevx! I added you to the org so CI runs/you can make a branch directly on the repo easier |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #275 +/- ##
==========================================
+ Coverage 72.59% 72.94% +0.34%
==========================================
Files 79 79
Lines 3653 3663 +10
==========================================
+ Hits 2652 2672 +20
+ Misses 867 861 -6
+ Partials 134 130 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
f8540b1
to
8a0227e
Compare
Thank you! That makes things easier |
8a0227e
to
ed32ece
Compare
Is it OK now? |
@aalekseevx looks great! mind writing a simple test that NACKs really get disabled. Just so in the future someone doesn’t break it then a merge from me! |
8edb5c7
to
af94ea2
Compare
Added some basic checks |
af94ea2
to
b8f3cb4
Compare
Allows user to selectively disable NACKs on some streams
b8f3cb4
to
1fc305b
Compare
It seems like 10ms is too small timeout for wasm arch, 50ms seems fine |
merged! Go ahead and tag w/e you want @aalekseevx |
Thank you! I hope I've done it right |
Description
This change allows users to pass custom stream filters to the nack interceptor in order to enable/disable nacks for certain streams based on the StreamInfo.