Skip to content

Improvement for issue #178 in beeswithmachineguns, no helpful informa…#199

Open
FliesLikeABrick wants to merge 1 commit into
newsapps:masterfrom
FliesLikeABrick:bugfix/issue-178-improvement
Open

Improvement for issue #178 in beeswithmachineguns, no helpful informa…#199
FliesLikeABrick wants to merge 1 commit into
newsapps:masterfrom
FliesLikeABrick:bugfix/issue-178-improvement

Conversation

@FliesLikeABrick

Copy link
Copy Markdown
Contributor

Improvement for #178 , no helpful information printed for _sting request failures, and is a fatal error. Modified to be non-fatal, prints error to stderr. This is not ideal, urllib.errors does not seem to exist on python2, was unable to find a way to catch HTTPError or URLERror specifically, so had to go with overly-broad exception catching

… information printed for _sting request failures, and is a fatal error. Modified to be non-fatal, prints error to stderr. This is not ideal, urllib.errors does not seem to exist on python2, was unable to find a way to catch HTTPError or URLERror specifically, so had to go with overly-broad exception catching
@cosmin

cosmin commented Nov 28, 2017

Copy link
Copy Markdown
Contributor

Perhaps look at switching to urllib3 instead? It’s the modern alternative and should make it easier to deal with this.

@FliesLikeABrick

Copy link
Copy Markdown
Contributor Author

@cosmin heh, I have been narrowly avoiding that since I wasn't sure about broaching the larger topic of python2 vs 3 support. I am happy to create a patch to change to urllib3 if we are ok with adding another non-stdlib dependency for python2. Thoughts?

@cosmin

cosmin commented Nov 29, 2017

Copy link
Copy Markdown
Contributor

I think that’s fine, better to add that dependency than deal with the urllib cluster

@FliesLikeABrick

Copy link
Copy Markdown
Contributor Author

ok, I will see work on this branch further (leaving the PR open) and take a shot at adapting to urllib3

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.

2 participants