Make concurrent requests whenever possible #1
Conversation
| if i == 0 { | ||
| cancel() | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
I'm not sure I got it right, but at this point isn't all the requests already made?
If that is true, then I think we can skip the check i == 0 to cancel()
There was a problem hiding this comment.
You are right, I think I did this to work around the existing tests; before this change, everything was executed in the order, so it didn't make sense to proceed after the first request failed.
I will see if I can infer the right behavior here.
There was a problem hiding this comment.
Got it, I couldn't see anything else so it looks good to me
There was a problem hiding this comment.
As far as I could find out, we only can halt the sequence when any required call failed, in order to keep the tests passing. I think it's a fair behavior.
Updated.
|
It is possible that they did not even consider this for foreseen complexity. And also it is definitely not sequential 😅 So better be named accordingly and added separately IMO. |
|
I think it would be a good idea, ideally it should be in the core framework. And in the worst case it might make us aware of some unexpected reasons on why it was not done As you achieved it, I think the honor to bring this up belongs to you 😁 |
When the sequential proxy is enabled the requests are not executed concurrently, but this branch modifies the behaviour by parsing the dependency first and dispatching only ready requests.
It can possibly make it to the origin, by adding some tests 🤔
@openrm/dev