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

new: use requests instead of urllib2. #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vaab
Copy link

@vaab vaab commented Dec 17, 2013

This avoids much of hassle with gzip encoding, multipart,
authentification, url encoding... which this module should avoid
managing because requests does it better and is widely supported and
tested.
This allows wikitools to concentrate on its main task.

Please don't accept this code for now: as there are no tests, I didn't check enough yet the multipart and encoding parts... This is a quick overview and I wanted to know if you are open to this type of changes.

I would have also other changes to propose (I wanted to apply some general PEP8 considerations), would you be open to such pull-request ?

Do you know if third party software are currently relying on your code ? I just want to get in hand some code to test that nothing is broken.

Thank you very much for your work on wiki-tools !

This avoids much of hassle with gzip encoding, multipart,
authentification, url encoding... which this module should avoid
managing because requests does it better and is widely supported and
tested.
This allows wikitools to concentrate on its main task.
@alexz-enwp
Copy link
Owner

This looks pretty interesting. I'm not normally a fan of adding dependencies on 3rd party software, but Requests looks pretty-well established (it's already installed by default on Wikimedia's Tool Labs platform)

In terms of testing, since everything calls APIRequest the same way, if it can log in and stay logged in, then everything else should work in theory. Perhaps another check of unicode handling. So a bare minimum test case would be

from wikitools import wiki, page
site = wiki.Wiki("https://en.wikipedia.org/w/api.php")
site.login("username", "password")
print site.isLoggedIn() # Should return True
p = page.Page(site, "Güçlükonak")
print p.title

The only thing this doesn't test is HTTP Auth (which isn't actually tested currently, since I don't have a place to test it).

I'd be interested in just about any improvements as long as they don't do anything like change public function inputs/outputs (I have sort of a grand plan for wikitools 2.0 that will change some of the less-optimal ones, but work on that is probably at least a few months away)

@fhocutt
Copy link
Contributor

fhocutt commented Jun 6, 2014

Using requests would improve security as well--requests automatically checks for ssl certificate validity, which urllib2 does not support at all.

alexz-enwp added a commit that referenced this pull request Dec 29, 2014
poster isn't available for Python3, so this is the only way to get
multipart for file upload

Heavily based on pull request #3 by vaab

Works for basic queries and login, untested on uploads and HTTP Auth

This also eliminates the ability to store the cookie data to a file.
It's not actually a requirement to do this, but that was a lot of
old, complicated code for a feature that (IMO) isn't very important.
@alexz-enwp
Copy link
Owner

As poster is unavailable for Python 3, I'm using requests in the Python 3 version I'm working on. The code here was useful in making the change - e5bc068 - I've gotten it working for uploads, but haven't been able to test HTTP Auth.

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