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

Simplify FeedLog table design #83

Closed
aronza opened this issue Aug 17, 2023 · 5 comments
Closed

Simplify FeedLog table design #83

aronza opened this issue Aug 17, 2023 · 5 comments
Assignees
Labels
decision-making Requires designing and deciding on a solution. enhancement New feature or request question Further information is requested

Comments

@aronza
Copy link
Contributor

aronza commented Aug 17, 2023

Describe the problem

In current database design, there is Feed table and FeedLog table with all the same columns except FeedLog has an extra historical_record_time column. If this is designed to track active and historical states of feeds, would it be easier to maintain only one feed table with record_time column?

This way the API can still serve the latest feeds when requested by querying for the feed's with latest record_time column per stable_id. Also maintaining these table schemas and inserting new data to them would be simpler.

I think we already have these table schemas diverged where Feed was added feed_contact_email at https://github.com/MobilityData/mobility-feed-api/blob/main/liquibase/changes/feat_15.sql but FeedLog doesn't have this column.

Proposed solution

Add record_time column to Feed table and remove FeedLog table

Alternatives you've considered

Existing schema

Additional context

This is not a blocker in my view but a nice discussion to have before going to production. If team feels there is use case for FeedLog that we can't handle easily with Feed table alone, I would be happy to keep the existing design.

@aronza aronza added the enhancement New feature or request label Aug 17, 2023
@welcome
Copy link

welcome bot commented Aug 17, 2023

Thanks for opening your first issue in this project! If you haven't already, you can join our slack and join the #mobility-database channel to meet our awesome community. Come say hi 👋!

Welcome to the community and thank you for your engagement in open source! 🎉

@indraneel indraneel added decision-making Requires designing and deciding on a solution. enhancement New feature or request question Further information is requested and removed enhancement New feature or request labels Aug 17, 2023
@emmambd emmambd moved this to 🔖 MobilityData Ready in Mobility Database API Aug 21, 2023
@emmambd
Copy link
Contributor

emmambd commented Aug 22, 2023

+1 to this simplification, particularly since we no longer anticipate as frequent updates to feed metadata when we stop switching URLs.

My only thought is that record_time may be unclear. Thoughts on updated_at instead? cc @qcdyx @cka-y

@cka-y
Copy link
Contributor

cka-y commented Aug 22, 2023

+1
our goal with the Feed table was that a single Feed in the table is related to a stable_id. Since we changed the definition of the Feed, I think the logs aren't required anymore. We could just update the Feed directly.

Also +1 for updated_at instead of record_time. However, we will lose historical records on what the update was (ex: the name of the feed was changed). Is that something we'd be ok with @emmambd ?

@emmambd
Copy link
Contributor

emmambd commented Aug 22, 2023

@cka-y Since we're still storing the historical versions of the feed itself (not its metadata) in v1/datasets/gtfs/{id} and the URL itself will no longer change for a feed, yes.

@kaushik12 kaushik12 assigned aronza and unassigned aronza Aug 23, 2023
@emmambd
Copy link
Contributor

emmambd commented Sep 11, 2023

@aronza After some discussion and MobilityData/mobility-database-catalogs#291, we no longer see a need for a feedlog or record. Plan currently is to

  • Remove Feedlog table
  • Remove feedlog endpoint

Users will get historical information on the dataset level, not about the feed.

Data Clinic is welcome to take this on!

@emmambd emmambd moved this from 🔖 MobilityData Ready to 📋 Two Sigma Data Clinic Backlog in Mobility Database API Sep 13, 2023
@aronza aronza closed this as completed Sep 20, 2023
@github-project-automation github-project-automation bot moved this from 📋 Two Sigma Data Clinic Backlog to ✅ Done in Mobility Database API Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
decision-making Requires designing and deciding on a solution. enhancement New feature or request question Further information is requested
Projects
Archived in project
Development

No branches or pull requests

4 participants