Skip to content

Added optional support for urllib2 instead of httplib2 - #8

Open
jcook793 wants to merge 7 commits into
embedly:masterfrom
Batterii:urllib2-option
Open

Added optional support for urllib2 instead of httplib2#8
jcook793 wants to merge 7 commits into
embedly:masterfrom
Batterii:urllib2-option

Conversation

@jcook793

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Author

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

@jcook793

Copy link
Copy Markdown
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
Copy Markdown
Author

Done

@jcook793

Copy link
Copy Markdown
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

2 participants