graphhopper / directions-api-clients

API clients for various languages for the GraphHopper Directions API
https://docs.graphhopper.com/
Apache License 2.0
30 stars 39 forks source link

Flag required fields as required on VRP API spec #19

Open teoguso opened 6 years ago

teoguso commented 6 years ago

The documentation of objects of the python client includes a detailed description of all the properties, but in all cases I've seen they are all optional. The problem is, this is not actually true, and submitting a request without some required properties will give an error. It would be nice to know exactly which properties are required, and in what cases.

For example, the Address object lists all properties as optional, but not providing a location_id will give an error. This pattern is true for almost all other objects in the library.

karussell commented 6 years ago

This might be a problem with the library creation. How would you label a property as required in python?

In the swagger config that is used to create the client we can say required:true.

teoguso commented 6 years ago

I'm not sure of how to tackle that with swagger, but maybe I should add that also in the original REST API docs (on the Graphhopper website) it's not clear what the minimal requirements are for a query to succeed. This might indicate an issue with the REST API docs and not specifically with the python client.

karussell commented 6 years ago

You are right in case of vehicle address specification this is missing:

https://graphhopper.com/api/1/docs/route-optimization/#services-or-shipments

but for the vehicle address (which is the same) we have it defined here (a bit below in a table):

https://graphhopper.com/api/1/docs/route-optimization/#vehicles

Have created an issue here:

https://github.com/graphhopper/directions-api/issues/75

teoguso commented 6 years ago

That's great, thanks! I still find the REST API docs a bit confusing in that it's not explicitly stated what the required minimal parameters are (see e.g. here to see how API docs declare the required and optional parameters for a given endpoint), but that might be a different issue, and I'm not sure where to post it.

karussell commented 6 years ago

Documentation issues can be posted or even better fixed here: https://github.com/graphhopper/directions-api/issues

Do you think if we would have a table for services and shipments with the required parameters this would fix this issue?

teoguso commented 6 years ago

That's great, I'll take a look at the main issues thread then.

I can tell you what feels right python-wise, that is to have required or optional function parameters clearly flagged. That would definitely solve this particular issue. Thanks!

michaz commented 6 years ago

In the swagger config that is used to create the client we can say required:true.

But we aren't. Changed the ticket title to make it clear that this is the problem which is under discussion here.