directions-api-clients icon indicating copy to clipboard operation
directions-api-clients copied to clipboard

Flag required fields as required on VRP API spec

Open teoguso opened this issue 8 years ago • 7 comments

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.

teoguso avatar Mar 29 '18 08:03 teoguso

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.

karussell avatar Mar 29 '18 08:03 karussell

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.

teoguso avatar Mar 29 '18 08:03 teoguso

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

karussell avatar Mar 29 '18 12:03 karussell

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.

teoguso avatar Mar 29 '18 16:03 teoguso

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?

karussell avatar Apr 07 '18 19:04 karussell

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!

teoguso avatar Apr 09 '18 07:04 teoguso

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.

michaz avatar Apr 23 '18 03:04 michaz