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

Fixed issue of profile_url not working as intended #346

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

cderk6
Copy link

@cderk6 cderk6 commented Nov 13, 2017

Http.request is returning the response to the final request, which in this case is the redirect. Therefore, the status in the event of a successful redirect would be '200' and profile_url would return None. Instead, the status of the previous response (pre-redirect) should be checked.

Copy link
Contributor

@ochawkeye ochawkeye left a comment

Choose a reason for hiding this comment

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

I for one would like to not see this change made to nflgame.teams as it would completely break how I use that in the API.

...
    ['KC',  'Kansas City',   'Chiefs',     'Kansas City Chiefs',   'KAN' ],
    ['LA',  'Los Angeles',   'Rams',       'Los Angeles Rams',     'LAR' ],
    ['SD',  'San Diego',     'Chargers',   'San Diego Chargers',   'SDG' ],
    ['LAC', 'Los Angeles C', 'Chargers',   'Los Angeles Chargers',       ],
    ['MIA', 'Miami',         'Dolphins',   'Miami Dolphins'              ],
...

@ochawkeye
Copy link
Contributor

I'm curious as well how the except would ever trigger here:

try:
    from collections import OrderedDict
except:
    from ordereddict import OrderedDict  # from PyPI

Isn't collections part of the standard library?

@cderk6
Copy link
Author

cderk6 commented Nov 20, 2017

My bad. That commit wasn't supposed to be in this pull request.

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.

None yet

2 participants