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

HSL: Fixed UnexpectedRedirectException related to incorrect API URL #143

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

msjoberg
Copy link
Contributor

This commit fixes bug #313 reported for Transportr. When running the HslProvider tests I also noticed another bug which I fixed.

…nd IllegalArgumentException in queryDepartures().
@grote
Copy link
Contributor

grote commented Feb 14, 2017

Thanks, looks good to me! :)

@schildbach
Copy link
Owner

Thanks! I had a slightly different fix on my master already. Can you look at 1b10571 and see if that fixes it for you too?

@msjoberg
Copy link
Contributor Author

@schildbach Hi, it's fixes one of the issues but not the other that I can see by running the HslProviderLiveTest:

	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:122)
	at de.schildbach.pte.dto.Location.<init>(Location.java:60)
	at de.schildbach.pte.dto.Location.<init>(Location.java:69)
	at de.schildbach.pte.dto.Location.<init>(Location.java:83)
	at de.schildbach.pte.HslProvider$3.onSuccessful(HslProvider.java:309)

You can see my fix for that here: https://github.com/schildbach/public-transport-enabler/pull/143/files#diff-f497cad9ef4e87811b6eff2ec1154ce2L309

@schildbach
Copy link
Owner

Can you rebase on current master and describe what the remaining change is fixing exactly? I don't see how this is related to the base URL problem (redirect).

@msjoberg
Copy link
Contributor Author

You're right: it's not related to the URL problem. It's described in the commit message, but not the pull request - sorry!

new Location throws an IllegalArgumentException without that change. It's such a trivial fix that it would perhaps be easier if you just did the change directly?

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.

3 participants