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

Use python-requests instead of urllib/urllib2/httplib #20

Closed
wants to merge 6 commits into from

Conversation

craigds
Copy link

@craigds craigds commented Mar 6, 2013

For much simpler/cleaner code.

I've signed the CLA, though I'm not exactly sure why it's necessary. This pull request contains vendored code authored by @kennethreitz and others, which is already Apache licensed, but they probably haven't signed the CLA (nor should they have to)

cf http://docs.python-requests.org/en/latest/

Note this is a fork of @intchanter's py3 work ( #18 ) so it depends upon that pull request being merged. This is because python-requests also requires python 2.6+, so this can't really be merged without his stuff.

(was intchanter#1 )

@lukeis
Copy link
Member

lukeis commented Mar 25, 2013

I don't agree with doing this change. The benefit (in only one location in the code base) of a cleaner api does not outweigh the maintenance burden of a 'third party' library. I think the preference in python in to use built-in libraries over third party, unless the benefit is significant.
I personally like the requests package and use it. I just don't think it's necessarily appropriate here.

@AutomatedTester opinions? :)

@davehunt
Copy link
Contributor

I agree with @lukeis, also please keep whitespace changes in separate commits.

@craigds
Copy link
Author

craigds commented Mar 25, 2013

Fair enough. We were getting some crazy urllib errors so this was a bit of
a knee jerk reaction to those. I spose we'll either maintain this fork or
we'll spend some time looking into the actual issue...
On 26/03/2013 5:49 AM, "Dave Hunt" notifications@github.com wrote:

I agree with @lukeis https://github.com/lukeis, also please keep
whitespace changes in separate commits.


Reply to this email directly or view it on GitHubhttps://github.com//pull/20#issuecomment-15405361
.

@lukeis
Copy link
Member

lukeis commented Mar 25, 2013

Thanks, if you can provide us a reproduction of the issue you saw we can certainly look into it to (feel free to log a bug in the issue tracker: https://code.google.com/p/selenium/issues/list ). For now, closing this issue.

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