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

Shows gobble data in the dashboard #943

Merged
merged 4 commits into from
Jan 30, 2024
Merged

Shows gobble data in the dashboard #943

merged 4 commits into from
Jan 30, 2024

Conversation

devinmatte
Copy link
Member

Motivation

Get the app in a state ready for gobble data

Changes

Only shows gobble data when manually navigating to a bus url with a date past the current maximum.

Nobody can reach gobble data through the UI, but could by changing startDate=2024-01-22 if they wanted to.

Adds a new warning banner to the bottom explaining that we parsed this data ourselves

Testing Instructions

Only shows gobble data when manually navigating to a bus url with a date
past the current maximum
@github-actions github-actions bot added backend Change to backend code frontend Change to frontend code labels Jan 29, 2024
@mathcolo
Copy link
Contributor

whoa! is this on beta?

@devinmatte
Copy link
Member Author

We have a version of this that includes commuter rail on beta. I can push this branch up instead if we want

@mathcolo
Copy link
Contributor

That would be great, would you mind?

@devinmatte
Copy link
Member Author

@mathcolo on beta now

if is_bus(stop_id):
return "daily-bus-data"
else:
return "daily-data"

Choose a reason for hiding this comment

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

the other folders in Events-live/ are daily-cr-data/ and daily-rapid-data/! daily-data is a subfolder of Events/

Copy link
Member Author

Choose a reason for hiding this comment

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

I have this in the other branch #898
I guess I can have all three in this branch

@@ -20,52 +20,70 @@
"CR-Needham",
"CR-Newburyport",
"CR-Providence",
"CR-Foxboro",

Choose a reason for hiding this comment

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

why remove foxboro?

Copy link
Member Author

Choose a reason for hiding this comment

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

We have no foxboro data yet since it's the event line, we can add it back later but for now there's nothing really to show

Copy link
Contributor

@idreyn idreyn left a comment

Choose a reason for hiding this comment

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

Lovely to see how well Gobble slots into our existing tools!

server/chalicelib/s3.py Outdated Show resolved Hide resolved
"stops": stop_layout[stop["id"]],
}
)
except KeyError:
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using it as a fallback, what if we always fetched from the API? My thinking is that if there's ever a problem with one of these files, it'll be easier to tell what caused it.

Copy link
Member Author

Choose a reason for hiding this comment

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

So this was only occurring for Worcester, but I gave this a try for everything, and then many stops on some lines returned less possible stop ids than generating the list from s3. Meaning gobble is detecting the trains at stops that the MBTA api doesn't think they will stop at. So really what we need here is a merge imo

server/scripts/generate_line_files.py Outdated Show resolved Hide resolved
@devinmatte devinmatte merged commit 5721452 into main Jan 30, 2024
4 checks passed
@devinmatte devinmatte deleted the gobble-bus-data branch January 30, 2024 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Change to backend code frontend Change to frontend code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants