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

Add customEndAtSeconds and uuid fields to Period #38

Draft
wants to merge 1 commit into
base: v2c
Choose a base branch
from

Conversation

tahini
Copy link
Collaborator

@tahini tahini commented Apr 20, 2021

In transition, those fields are being added to allow to customize when a
period should end and the uuid field is useful to save the period in the
database.

In transition, those fields are being added to allow to customize when a
period should end and the uuid field is useful to save the period in the
database.
@tahini
Copy link
Collaborator Author

tahini commented Apr 20, 2021

DO NOT MERGE YET I'm putting the patch here, but I'd like to add some unit tests around it and wait to see if the uuid field is absolutely necessary (since it goes from database to the cache, but it does not need to be the other way around...)

@greenscientist
Copy link
Contributor

@tahini Is this still relevant ?

@tahini
Copy link
Collaborator Author

tahini commented Feb 22, 2022

Still relevant I guess, but didn't have time to look into it. Is the period even necessary? Or could we just keep trips hours?

@tahini tahini marked this pull request as draft February 22, 2022 18:32
@greenscientist
Copy link
Contributor

@tahini, we should just do this. Having the capnp file out of sync does not make sense. Either we remove them from the transition side or we merge this in TrRouting.

@tahini
Copy link
Collaborator Author

tahini commented Apr 30, 2024

@greenscientist There's still the question of what to do with those fields. Is there any use case to have the capnp file have more data that what is needed in trRouting? Or do we consider those files are "just" a cache and can/should contain the minimal information required to run? (especially if we plan to store them along with calculations for later re-use, it goes towards minimal information)

@greenscientist
Copy link
Contributor

From the TrRouting perspective, this file should be in sync.
The question to remove the field should be in the Transition side of things. So if we don't settle this idea shortly, I propose we sync the files and figure it out later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants