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 '/' to Metacalculus questions url to remove redirect #428

Merged
merged 2 commits into from
Nov 3, 2020

Conversation

zmpster
Copy link
Contributor

@zmpster zmpster commented Nov 2, 2020

Closes #363. There wasn't a great way to test this without exposing too much from the Metaculus class, thought if you disallow redirects, as I have done, but don't add the /, the tests will fail.

@zmpster zmpster requested a review from brachbach as a code owner November 2, 2020 21:12
Copy link
Member

@brachbach brachbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Actually my current guess is that we do still want to allow redirects (remove allow_redirects=False) so that this code will still work if Metaculus ever changes their API URL in the future and adds a redirect to the new URL

Do you think that makes sense @zmpster?

@zmpster
Copy link
Contributor Author

zmpster commented Nov 3, 2020

Yeah, that makes sense. I was thinking disallowing redirect explicitly was a bit too cute. Removed that option!

Copy link
Member

@brachbach brachbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making that change!

I tested locally, everything works :)

merging

@brachbach brachbach merged commit eb1ed9e into oughtinc:master Nov 3, 2020
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.

Remove unnecessary redirect in Metaculus API requests
2 participants