fix(core-utils): optimize flex combination generation #747
Conversation
This accomplishes two things. First, it only allows FLEX_DIRECT on its own, without transit or anything else. Although this should work, OTP has a bug that causes issues with this configuration. It doesn't make sense to combine them anyway. Second, it makes FLEX (access/egress) *only* comaptible with TRANSIT. As far as I can tell, OTP doesn't support FLEX combined with bikeshare or anything else. This should reduce the number of queries we send to OTP, which will improve performance. Lastly, it improves the test output to make it easier to read.
|
A few things
I am hesitant to make these fundamental changes to the core of otp-ui when this is a performance problem that needs to be solved within OTP. Reducing the number of requests to the server isn't going to help performance, especially when we support load balancers |
|
Not sure about 1. Will have to investigate more. For (2) I think this is a good approach, since it always felt like a hack to me to bundle flex with scooters and bike share. It really feels like its own thing. Especially when FLEX_DIRECT has to be treated separately. As for performance, I sort of agree, but it is actually quite expensive to spin up more of these large instances that are used for flex-enabled OTP. We need to approach the performance problem from every angle, and especially this one if the extra requests aren't yielding any different results. |
|
I think we've confirmed now that FLEX_DIRECT should also be alone. It doesn't make sense to add other modes since it's specifically door to door flex. For ACCESS and EGRESS, we might want to combine it with other access and egress modes. The unit tests previously didn't differentiate between flex_direct and flex_access/egress, because the function itself didn't know the difference. Now it does, which is why I modified the unit tests as well. |
This accomplishes two things. First, it only allows FLEX_DIRECT on its own, without transit or anything else. Although this should work, OTP has a bug that causes issues with this configuration. It doesn't make sense to combine them anyway.
Second, it makes FLEX (access/egress) only comaptible with TRANSIT. As far as I can tell, OTP doesn't support FLEX combined with bikeshare or anything else. This should reduce the number of queries we send to OTP, which will improve performance.
Lastly, it improves the test output to make it easier to read.