Add optional "forceSequential" flag to batch request payload. #86
Conversation
test/internals.js
Outdated
| return sequentialHandler.callCount; | ||
| } | ||
|
|
||
| await awaitDelay(Math.floor(Math.random() * 100)); |
There was a problem hiding this comment.
I think in a perfect world the sequential handlers would count down: the first 40ms, the second 30ms, etc.
There was a problem hiding this comment.
Yeah, that's a good idea. I just wanted to make sure the first definitely was by far the longest, and added some response time jitter just to keep it interesting :P
The main portion was the first -> second calls since .callCount would definitely 100% be incremented by then. I'll go ahead and do the delay decrement though. It's a good idea 👍
There was a problem hiding this comment.
Oh!! I actually missed that finer point. I think ultimately we're good either way :)
|
Had a suggestion, but overall this looks good to me! |
|
Thanks for the input :) I went ahead and pushed up a commit to simplify the sequentialHandler in the test routes to do the more straightforward and just as effective decrementing delay |
cadecairos
dismissed
their stale review
CI is failing after an incomplete merge conflict resolution
|
@WesTyler My apologies, but I missed a semicolon when resolving the merge conflict via the GitHub editor. Could you add a commit to your branch that fixes that? |
|
@WesTyler ping. |
This adds the ability to force a specified batch of requests to be run sequentially even without pipelining. This is done on a per-request basis, as proposed in #85 .