Default sending_queue.batch.sizer to items when sending_queue.sizer is not set #14438
Conversation
…s unset Signed-off-by: Israel Blancas <iblancasa@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #14438 +/- ##
==========================================
+ Coverage 91.80% 91.81% +0.01%
==========================================
Files 677 677
Lines 42608 42611 +3
==========================================
+ Hits 39116 39125 +9
+ Misses 2429 2425 -4
+ Partials 1063 1061 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
| // 1. the batch sizer is not set and | ||
| // 2. the batch section is nonempty, | ||
| // then use the same value as the queue sizer if set, otherwise default to items. | ||
| if !conf.IsSet("batch::sizer") && conf.IsSet("batch") && conf.Get("batch") != nil { | ||
| if conf.IsSet("sizer") { | ||
| cfg.Batch.Get().Sizer = cfg.Sizer | ||
| } else { | ||
| cfg.Batch.Get().Sizer = request.SizerTypeItems | ||
| } |
There was a problem hiding this comment.
I think this logic is not correct, because if the caller has set a different default for the batch sizer this overwrites that.
I am not sure what happens here, because based on NewDefaultQueueConfig the default should be items. Can you add a test that starts from NewDefaultQueueConfig and proves this bug?
There was a problem hiding this comment.
You are totally right. Though I reproduced the issue but was wrong.
After doing another review, found the issue is actually fixed. Will close this PR and post a comment in the ticket.
Thanks!
2 participants
Link to tracking issue
Fixes #14435