Migrate to GTFS-rt predictions and locations feeds #743
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Coverage of commit
|
panentheos
reviewed
Mar 26, 2024
lib/predictions/predictions.ex
Outdated
| @@ -10,11 +10,23 @@ defmodule Predictions.Predictions do | |||
| predictions = | |||
| feed_message["entity"] | |||
| |> Enum.map(& &1["trip_update"]) | |||
| |> Enum.filter( | |||
There was a problem hiding this comment.
nit: We could use Stream here for efficiency, and/or combine this with the following reject.
There was a problem hiding this comment.
Ah true. Might as well use the Stream versions of most of the rest of these chained operations too. Any opposition to that?
lib/predictions/predictions.ex
Outdated
| stops_away: stop_time_update["stops_away"], | ||
| stopped?: not is_nil(vehicle_location) and vehicle_location.status == :stopped_at, | ||
| vehicle_at_predicted_stop?: | ||
| not is_nil(vehicle_location) and stop_time_update["stop_id"] == vehicle_location.stop_id, |
There was a problem hiding this comment.
Since we always look at these two values together, what do you think about combining them into a single flag like stopped_at_predicted_stop?, which would be a drop-in replacement for the previous stops_away == 0?
test/engine/predictions_test.exs
Outdated
| @@ -65,6 +65,8 @@ defmodule Engine.PredictionsTest do | |||
| assert :ets.info(predictions_table)[:size] == 2 | |||
| [{{"stop_to_remove", 0}, []}] = :ets.lookup(predictions_table, {"stop_to_remove", 0}) | |||
|
|
|||
| :ets.lookup(predictions_table, {"stop_to_update", 0}) | |||
There was a problem hiding this comment.
This looks like it was left in accidentally.
| nil -> [] | ||
| stops -> [stops_away: stops, boarding_status: "Stopped #{stops} stop away"] | ||
| end | ||
|
|
There was a problem hiding this comment.
This still seems like a useful bit of abstraction over some common data patterns. If we just update it to use the new flag(s), we could probably avoid modifying some of these tests.
Coverage of commit
|
Coverage of commit
|
panentheos
approved these changes
Mar 28, 2024
Co-authored-by: Brett Heath-Wlaz <[email protected]>
Coverage of commit
|
…bta/realtime_signs into pk/migrate-from-rtr-to-gtfs-rt
Coverage of commit
|
Coverage of commit
|
|
@panentheos When if you have a minute, could I get another pass at this one?
|
panentheos
approved these changes
Jun 13, 2024
lib/predictions/last_trip.ex
Outdated
| "Green-D", | ||
| "Green-E", | ||
| "Mattapan" | ||
| ] and &1["trip"]["schedule_relationship"] != "CANCELED") |
There was a problem hiding this comment.
nit: We might want to split this route list into a helper, e.g. relevant_rail_route?, since we intend for both code paths to use the same list.
Coverage of commit
|
PaulJKim
added a commit
that referenced
this pull request
Aug 2, 2024
|
Restoring the branch so we can deploy it to dev-green while we investigate what's going on with BRD predictions and passthrough announcements |
PaulJKim
temporarily deployed
to
dev-green-linux
GitHub Actions
Inactive
PaulJKim
temporarily deployed
to
dev-green-linux
GitHub Actions
Inactive
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Asana Ticket: Rearchitect RTS off of RTR
This change adjusts the predictions parsing logic to allow RTS to use the GTFS-rt feeds instead of the RTR feeds. The locations parsing logic doesn't need to be change because the GTFS-rt locations feed more closely mirrors the RTR locations feed.
The discrepancies in what fields are provided in the GTFS-rt feed compared to the RTR feed necessitated some slight logic changes in the main predictions flow. Most notably, the
stops_awayfield is not in the GTFS-rt field, but we are able to use locations data to mimic howstops_away = 0is determined as an alternative.Changes can be tested locally against the GTFS-rt production feeds by using the following URLs for the
TRIP_UPDATE_URLandVEHICLE_POSITIONS_URLenv variables respectivelyhttps://cdn.mbta.com/realtime/TripUpdates_enhanced.jsonhttps://cdn.mbta.com/realtime/VehiclePositions_enhanced.jsonBefore this is merged/deployed, we will want to put up a Terraform PR to change the values of the aforementioned env variables first. Once that is merged and applied, then we can re-deploy RTS with the new logic.