Skip to content

Use HTTPS, fix v2 API, fix error logging#20

Merged
steffex merged 4 commits intosteffex:masterfrom
janverb:master
Nov 5, 2020
Merged

Use HTTPS, fix v2 API, fix error logging#20
steffex merged 4 commits intosteffex:masterfrom
janverb:master

Conversation

@janverb
Copy link
Copy Markdown
Contributor

@janverb janverb commented Aug 21, 2020

These are small fixes for a few different issues. Mainly:

  • It's better to use HTTPS so people can't snoop on the API key and user addresses.

  • An earlier commit accidentally broke the formatting of the URL for the v2 API.

  • handleresponseerror() wasn't being used because it didn't expect an HTTPError.

Comment thread pyPostcode/__init__.py Outdated

def handleresponseerror(self, status):
if status == 401:
if status in {401, 403}:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Dictionary? Why not a list?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No particular reason when it's so small, I just have a habit of using sets for lookups because they're fast.
I'll change it to a list to be less confusing.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh it creates set. Was not aware of this syntax, but that's perfect.

- v2 returns a status code of 403 rather than 401.
- The format string for v2 didn't work.
It could do the wrong thing in some implementations of Python.
urlopen raises a HTTPError upon failure, which didn't go through
handleresponseerror().

The original handling is left in case there are obscure conditions
where urlopen doesn't raise a HTTPError.
@steffex steffex merged commit 40be282 into steffex:master Nov 5, 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.

4 participants