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

Added optional support for urllib2 instead of httplib2 #8

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

Conversation

jcook793
Copy link

I'm working on a project that uses Google App Engine which requires using urllib, urllib2 or httplib for outbound HTTP requests. In this pull request I added support for urllib2 by passing the optional "use_urllib2" parameter when creating an Embedly client object.

Note that I essentially run all the tests twice, once using httplib2 and once using urllib2. It's kind of goofy but I couldn't think of a better way of doing it.

@screeley
Copy link
Contributor

Hey,

This is on my radar, but I think there is a cleaner way by making it easy to subclass the client. I.e:

from embedly import Embedly

class Urllib2Embedly(Embedly):

    def request(url):
        #do urllib2 stuff here. 

I can image other people wanting to use something other than httplib or urllib2.

Thanks!

Sean

@jcook793
Copy link
Author

That's a good idea. I'll work on that and submit another pull request.

@jcook793
Copy link
Author

This implementation doesn't use inheritance, more of a mixin-type approach. Let me know if you hate it. :)

Also let me backout that version # change (using that for the buildout in our GAE project ) so it doesn't get applied to this pull request.

@jcook793
Copy link
Author

Done

@jcook793
Copy link
Author

Turns out I didn't understand pull requests (or the definition of "done") as much as I thought I did - now I'm done.

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