Nav 131 extend query config for raptor #100
Merged
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
…onService interface.
…m to reduce code duplication.
…uting controller.
…and gtfs defaultroutetype.
… stop time array with specific query settings.
…ys for a given date.
|
@munterfi: PR is finished for review and potential merger. Only thing one could still consider with a slight potential for adding too much complexity is considering the accessibility of stops in the routing process as well. This would require reworking the raptor builder and router a bit so that stops would also have a flag for accessibility. |
munterfi
requested changes
Aug 19, 2024
There was a problem hiding this comment.
Thanks @clukas1, nice work!
The overall concept looks solid and is close to being ready for merge. I have a few minor concerns that I'd like to discuss:
RoutingInfovs.ScheduleInfo: Not consistent where this should live (ScheduleInfomrationServicevs.ConnectionRoutingService) between the layersserviceandapp.- Why is there and option to have no travel modes? Do you know sources other than GTFS that are not providing this? In order to be generic we could keep it. But in our case this will always be
supportsTravelModes=true. - Configuration specific raptors and trip masks: Could you profile and benchmark to see how this is going to impact our memory footprint and performance?
src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsTripMaskProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsTripMaskProvider.java
Outdated
Show resolved
Hide resolved
- Rename endpoint for RoutingInfo from "/routing/" to "/routing", so it appears on top in the Swagger UI.
…name routerInfo to scheduleInfo.
…aptor' into NAV-131-Extend-QueryConfig-for-Raptor # Conflicts: # src/main/java/ch/naviqore/app/controller/RoutingController.java # src/main/java/ch/naviqore/app/dto/ScheduleInfo.java
…d to queryconfig class.
… router controller.
…rce coordinates are identical.
…d target coordinates.
…nd BikeInfo to Trip DTO.
|
@munterfi added small issues NAV-141 (add check for same coordinates in routing controller) and NAV-148 - add accessibility information and bikes allowed to trip dto and route transportmode enum to route dto. |
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.
@munterfi this is part one of my query config issue. I thought I'll initiate the review process early to unlock the service for further extension (less merge conflicts).
So far I've extended everything on the app / spring endpoint side to handle extra query args as I would expect. Down to creating the service QueryConfig object. In a separate branch, I will use this QueryConfig object to build a raptor query config object and mask my stop times accordingly + tests.
A further issue I will tackle is creating multiple versions of my zürich trams gtfs schedule where I add accesibility/bike information for testing.