Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to add endpoint slug as argument #218

Closed
nilsnolde opened this issue Mar 4, 2019 · 5 comments
Closed

Ability to add endpoint slug as argument #218

nilsnolde opened this issue Mar 4, 2019 · 5 comments

Comments

@nilsnolde
Copy link
Contributor

nilsnolde commented Mar 4, 2019

We face the problem, that ORS provides different slugs for the Matrix endpoint before the endpoint name, depending on the service provider: no slug for hosted, /openrouteservice-5.0/ for local server, /ors/ for Docker container. Admittedly, that's not ideal.
However, the same problem applies to OSRM, in case one would like to use vroom with Mapbox OSRM matrix endpoint, which uses /directions-matrix/ instead of /table/.
What do you think about :

  • additional argument specifying a base_url/slug
    or
  • extending the host argument to provide a full base_url (probably more problematic?)

Or maybe you have a better idea?

EDIT:
hmm, to be fully flexible one would also need to be able to set query parameters for API keys. Is that even smth you'd want or that you see within the scope of vroom?

@nilsnolde nilsnolde changed the title Ability to add slugs to host Ability to add endpoint slug as argument Mar 4, 2019
@jcoupey
Copy link
Collaborator

jcoupey commented Mar 4, 2019

Adding parameters for API keys might make sense at some point, but I think we should focus first on dealing with the required changes in URLs so that everything works with a local routing server.

Full URL for matrix and routing requests used inside VROOM:

  • OSRM: host:port/service/v1/profile/...
  • ORS 5: host:port/base/v2/service/profile

where the base part for ORS can be empty, openrouteservice-5.0 or ors. What about adding a flag similar to -a and -p to optionally set this value? In order to solve a problem containing vehicles with driving-car profile using a local ORS instance with docker, one would use:

-a driving-car:localhost -b driving-car:ors -p driving-car:8080

@nilsnolde
Copy link
Contributor Author

nilsnolde commented Mar 4, 2019

but I think we should focus first on dealing with the required changes in URLs

Yes, agreed. Although in the meantime, we can resolve this issue with #432 on our end. At least for the local case, Docker and system installations would both work with /ors/. Which means it could be hard-coded in the ORS wrapper and sort of resolve this issue.

What about adding a flag similar to -a and -p to optionally set this value?

That's what I thought as well. If remote services should be supported, this solution is still needed and valid.

@jcoupey
Copy link
Collaborator

jcoupey commented Mar 4, 2019

Docker and system installations would both work with /ors/

Great! The plan here was to support ORS as of version 5 in the next release (via #215), not bothering with backward compatibility with the ORS 4.* series. So I'd be inclined to keep the hard-coded /ors/ part of the URL for our next release, which would mean that everything works out-of-the-box for a local routing server running ORS 5.1.

Then we can focus on the remote service part (based on ORS or OSRM), which would include adding a parameter for an API key, while removing or changing parts of the URL as required.

What do you think?

@nilsnolde
Copy link
Contributor Author

Sounds great @jcoupey! Thanks for running such an open and collaborative project!

@jcoupey
Copy link
Collaborator

jcoupey commented Mar 5, 2019

OK, so closing here as it's probably clearer to have a dedicated ticket for the API key part. @nilsnolde feel free to open one if you have time, I'll flag it for the v1.5 milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants