Fix route v8 multiple via points #76
Conversation
Fix route v8 multiple via points
| @@ -448,7 +448,7 @@ def route_v8( | |||
| via_keys = [] | |||
| if via: | |||
| for i, v in enumerate(via): | |||
| key = str.format("{0}{1}", "via", i) | |||
| key = str.format("{0}{1}_", "via", i) | |||
There was a problem hiding this comment.
What is the purpose of using _ here?
There was a problem hiding this comment.
If you have 11 via points, you will have via1 and via10 in data and manipulation_keys
The first url built in __get will contain: via1=...&via10=..
Then we loop to replace those manipulation_keys (via1 and via10) by the one needed by HERE (via):
url.replace("via1", "via")->via=...&via0=...(it also replaces the first part ofvia10)url.replace("via10", "via")->via=...&via0=...(it does nothing, so the second via point is not given through theviaquery parameter, and so HERE does not take the second via point into account)
By creating via1_ and via10_ keys instead:
given the url containing via1_=...&via10_=..
url.replace("via1_", "via")->via=...&via10_=...(it replaces only the first key)url.replace("via10_", "via")->via=...&via=...(it replaces the second key)
There was a problem hiding this comment.
Thanks for the clarification 👍 changes look good to me.
There was a problem hiding this comment.
Would you mind bumping the version of codecov to make the CI happy?
There was a problem hiding this comment.
Thanks for bumping the codecov version, it seems like your test is failing. Let's fix it and then we can merge.
There was a problem hiding this comment.
Responses requires Python 3.8 or higher as I see, can you remove the previous versions from CI config?
There was a problem hiding this comment.
do you want me to remove it from ci.yml and .travis.yml, but keep python_requires=">=3.5" in setup.py?
There was a problem hiding this comment.
Yes please, 3.5 has already reached end of life. It's a good time to stop supporting it. Please update the setup.py too.
|
It looks like the merge has failed and the fix is not part of the last release |
Regarding
route_v8and the use ofviaparameter:When adding more than 10 via points, the last ones are ignored.
It comes from the fact that in that case, some
keys_for_manipulationare substrings of others (ex:"via1"and"via10").When you replace the first one in the url, it also replaces a part of the second one, and so the replace does not work as expected (you end up with a
via0=in the final url)I just ensured that first keys can not be substrings of last ones.