-
Notifications
You must be signed in to change notification settings - Fork 4
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
Booking API (originally by MaaS Global) #10
Conversation
Co-authored-by: laurisvan Co-authored-by: Feijk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of suggestions to align this with the MaaS API, schema.org and FIWARE. Also raised some points of discussion.
booking.yaml
Outdated
openapi: 3.0.0 | ||
servers: [] | ||
info: | ||
version: 0.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be version: 0.1.0
, otherwise http://editor.swagger.io complains with an error.
booking.yaml
Outdated
description: > | ||
This is a API specification of REST endpoints that a Transport Service | ||
Provider (TSP) should implement to receive messages from MaaS Operator(s). It is written | ||
in machine readable [Swagger](http://swagger.io/) format, so that API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Sagger format has since been renamed "OpenAPI Spec"
booking.yaml
Outdated
TSP. | ||
externalDocs: | ||
description: Booking scenarios | ||
url: 'https://github.com/maasglobal/maas-tsp-api/blob/master/specs/Booking.md' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this also be added to this repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
booking.yaml
Outdated
- name: Listing | ||
description: >- | ||
Before a booking can be made via a TSP, available options at a given | ||
location can be listed as follows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about naming this a "Quote" instead? "Listing" is more general and less descriptive, but I think this is essentially for getting a quote which can later be booked?
booking.yaml
Outdated
description: >- | ||
In the future MaaS will implement machine-readable APIs that a TSP may use | ||
to update GTFS route information, API keys and other information that is | ||
needed for communicating between MaaS and a TSP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd leave those TODO
tags out of this PR and create separate issues for them.
- startTime | ||
- endTime | ||
place: | ||
type: object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can reuse Location and StopLocation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. How should we separate those definitions, so they can be shared between APIs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enhancement opened:
#11
booking.yaml
Outdated
time: | ||
description: >- | ||
A UTC timestamp (number of milliseconds in a Date object since January | ||
1, 1970, 00:00:00) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We see more APIs switch to ISO8601 for times as it makes developing and making sense of looking at API responses much simpler, so I suggest to switch to that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, ISO8601 is good. Not sure why epoch was chosen, but we seem to have it in a lot of places internally, including object sub-fields (JSONB). This adds a bit of difficulty when working with the data.
booking.yaml
Outdated
A UTC timestamp (number of milliseconds in a Date object since January | ||
1, 1970, 00:00:00) | ||
duration: | ||
description: A duration of some time (relative to time) in milliseconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Milliseconds seems unnecessary high precision, would recommend seconds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jep, milliseconds is a bit difficult to work with. I am not sure why it was in the original spec.
minimum: 0 | ||
fare: | ||
description: Arbitrary fare data that MaaS will use internally | ||
type: object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to have a schema or recommendation in place for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. Can we open an enhancement issue for the fare schema?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, let's make this an enhancement issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enhancement issue created
#13
description: Arbitrary fare data that MaaS will use internally | ||
type: object | ||
mode: | ||
description: The type of the leg MaaS uses to identify the leg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can reuse ModeInfo and particularly the "mode identifiers" to have a defined hierarchy of supported modes and make it consistent across APIs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can we split these out, so they can be shared between APIs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make this an enhancement issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related to #11
Co-authored-by: Tomi Niittumäki [email protected] Co-authored-by: Lauri Svan [email protected]
Co-authored-by: Lauri Svan [email protected]
@nighthawk, I have made agreed changes and opened related enhancement issues #11 and #13. Please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Booking API that can be used by Transportation Service Providers to allow MaaS Operators to make bookings in a given transportation service network.