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

Clarify which vehicles to include in the /vehicles endpoint #882

Conversation

jiffyclub
Copy link
Contributor

MDS Pull Request

Explain pull request

The description of the Provider vehicles endpoints was never really updated in the MDS 1 -> 2 transition so this rearranges and clarifies some things now that there are two vehicle information endpoints, one for static information like vehicle/propulsion type and another for realtime status. These changes are mostly rearranging/clarifying, but there is one
major addition to the description of the /vehicles endpoint describing that when called without a device ID it is expected to return every vehicle ever deployed in a jurisdiction. This closes #879.

Is this a breaking change

  • No, not breaking. This makes no changes to the spec format, only clarifies content expectations.

Impacted Spec

Which spec(s) will this pull request impact?

  • provider

Additional context

See also #879.

The description of the Provider vehicles endpoints was never
really updated in the MDS 1 -> 2 transition so this rearranges
and clarifies some things now that there are two vehicle information
endpoints, one for static information like vehicle/propulsion type
and another for realtime status. These changes are mostly
rearranging/clarifying, but there is one major addition to the
description of the `/vehicles` endpoint describing that when
called without a device ID it is expected to return every vehicle
ever deployed in a jurisdiction.
@jiffyclub jiffyclub requested a review from a team as a code owner October 18, 2023 19:42
@schnuerle schnuerle linked an issue Oct 19, 2023 that may be closed by this pull request
@schnuerle schnuerle added the Provider Specific to the Provider API label Oct 19, 2023
@schnuerle schnuerle added this to the 2.0.1 milestone Oct 19, 2023
@schnuerle schnuerle added the documentation documentation change can be for code and/or markdown pages label Oct 19, 2023
@schnuerle
Copy link
Member

schnuerle commented Dec 5, 2023

@jiffyclub do you think you can present some slides to explain this change at this Thursday's working group meeting? If so it can be the main agenda item.

@jiffyclub
Copy link
Contributor Author

Yes, can do!

@@ -228,7 +225,21 @@ See [Responses][responses], [Bulk Responses][bulk-responses], and [schema][schem

### Vehicle Status

The `/vehicles/status` endpoint returns the specified vehicle (if a device_id is provided) or a list of known vehicles. Contains specific vehicle status records that are updated frequently.
The `/vehicles/status` endpoint is a near-realtime endpoint and returns the current status of vehicles in an agency's [Jurisdiction](/general-information.md#definitions) and/or area of agency responsibility. All vehicles that are currently in any [PROW](/general-information.md#definitions) state [`vehicle_state`][vehicle-states] should be returned in this payload. Since all states are returned, care should be taken to filter out states not in the [PROW](/general-information.md#definitions) if doing vehicle counts. For the states `elsewhere`, `removed`, and `missing`, which include vehicles not in the [PROW](/general-information.md#definitions) but provide some operational clarity for agencies, these vehicles must only persist in the feed for 90 minutes before being removed (and should persist in the feed for at least 90 minutes).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"these vehicles must only persist in the feed for 90 minutes before being removed (and should persist in the feed for at least 90 minutes)" is the same as "these vehicles must persist in the feed for exactly 90 minutes", which sounds a bit strict, honestly. (I suppose the existing language comes across this way as well, but less explicitly.)

I really don't want to be called out-of-compliance because a vehicle was present for 91 minutes because some backend process got delayed by 1 minute. Technically enforcing such a TTL is not much more difficult than one with a grace period, but maybe allowing for things to be present for, say, up to 120 minutes will save some annoying discussions. I can't imagine what logic a consumer might have that would be thrown off by such a grace period (maybe someone will think of something though).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We didn't come to a final decision on this in the meeting today, but people seemed open to softening this language a bit. Is "these vehicles must persist for no more than 90 minutes" the spirit of the original language?

I'll also mention that we do not currently return elsewhere vehicles at all (they're not in whichever jurisdiction, so returning "the status of all vehicles currently in the jurisdiction" would seem to naturally exclude them, the elsewhere update would be available in events/recent if people want to see those in near-realtime), so a minimum guarantee for persisting such vehicles would be a change.

@jiffyclub
Copy link
Contributor Author

Based on discussion on today's MDS call I'll update this to specify:

  • /vehicles must return all vehicles deployed in a region in the last 30 days
  • /vehicles/{device_id} must return vehicle information for all device IDs present in MDS data available from other endpoints regardless of when the device was deployed. If the vehicle information has changed over time the most recent information should be returned.

- Specify that /vehicles called without a device ID should return
  all vehicles deployed in a region in the last thirty days
- Specify that vehicle information about all device IDs present
  in MDS data for a region must be accessible via /vehicles/device_id
  regardless of when the vehicle was deployed
@jiffyclub jiffyclub mentioned this pull request Dec 11, 2023
@schnuerle schnuerle merged commit 3a71f44 into openmobilityfoundation:dev Dec 13, 2023
1 check passed
@jiffyclub jiffyclub deleted the vehicles-which-vehicles-to-include branch December 13, 2023 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation documentation change can be for code and/or markdown pages Provider Specific to the Provider API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

What vehicles are included in the MDS 2.0 /vehicles endpoint?
3 participants