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

bannedAgencies parameter does not work as expected for multi agency feeds #2057

Closed
johannilsson opened this Issue Jul 26, 2015 · 6 comments

Comments

Projects
None yet
4 participants
@johannilsson
Copy link
Contributor

johannilsson commented Jul 26, 2015

For feeds that has one or more agencies the bannedAgencies does not work as expected, either the whole feed is banned or the passed agency is not banned. This happens because the lookup is done towards the feed id rather than the actual agency id in RoutingRequest.tripIsBanned.

@kirkhus

This comment has been minimized.

Copy link

kirkhus commented Nov 18, 2015

Please, can we get this patch merged into the master branch, or at least a comment as to why not?

I'm running into exactly this issue at my workplace now. We have multiple agencies in our GTFS setup and bannedAgencies only works with the default agency that is first in the agency.txt file.

I did the exact same change on my local OTP instance and this fixed the issue with multiple agencies.

@johannilsson

This comment has been minimized.

Copy link
Contributor Author

johannilsson commented Dec 1, 2015

@kirkhus We've been running with this patch for quite some time now and haven't seen any side effect, so from what I can tell with our data and setup it's safe to run.

We also have a few other fixes for multi agency feeds in the branch called develop in the fork on my account if that's of interest.

@abyrd

This comment has been minimized.

Copy link
Member

abyrd commented Dec 1, 2015

Thanks for your comment @johannilsson, I wasn't completely sure about the stability of these changes. @kirkhus sorry for the delay, when I saw your message go by I was concerned about the impact on realtime messages but I believe everyone using real time updates is in agreement with Johan's method. I will merge this as soon as I do one last quick review, probably today.

@johannilsson

This comment has been minimized.

Copy link
Contributor Author

johannilsson commented Dec 1, 2015

@abyrd Maybe worth adding that I've only done basic testing with this patch alone, in production we're running with the changes in #1999. But I added this as a separate PR sense this change can be applied separate from that fairly big patch and would work for the basic cases. But to get RT to work for multi agency feeds I'm afraid the other PR or something similar is needed.

@kirkhus

This comment has been minimized.

Copy link

kirkhus commented Dec 1, 2015

We have been running this change about 10 days now and can verify that it is working fine and lets us use the "bannedAgencies" parameter when we have multiple agencies.

@andreyz

This comment has been minimized.

Copy link
Contributor

andreyz commented Dec 10, 2015

Ping @abyrd. Anything else is required for this to be pulled in?

@abyrd abyrd closed this in d59e320 Dec 22, 2015

abyrd added a commit that referenced this issue Dec 22, 2015

Merge pull request #2058 from johannilsson/jn/fix-banned-agencies-for…
…-multi-agency-feeds

Fix banning of agencies in multi agency feeds, resolves #2057

flibbertigibbet pushed a commit to flibbertigibbet/OpenTripPlanner that referenced this issue Jan 25, 2016

Fix banning of agencies in multi agency feeds, resolves opentripplann…
…er#2057

Change to lookup the agency id through the route instead of the trips id¬
field which contains the feed id in multi agency feeds.¬

flibbertigibbet pushed a commit to flibbertigibbet/OpenTripPlanner that referenced this issue Jan 25, 2016

Merge pull request opentripplanner#2058 from johannilsson/jn/fix-bann…
…ed-agencies-for-multi-agency-feeds

Fix banning of agencies in multi agency feeds, resolves opentripplanner#2057
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.