Update pysimplehttp #75

Merged
merged 1 commit into from Sep 10, 2012

Conversation

3 participants
Contributor

mynameisfiber commented Sep 10, 2012

This pull request updates the pysimplehttp library and adds the
BaseReader class to help with simplehttp python development.

setup.py
+ 'ujson',
+ 'host_pool',
+ ],
@ploxiln

ploxiln Sep 10, 2012

Contributor

I'm curious, what happened to requires, was it redundant?

@mynameisfiber

mynameisfiber Sep 10, 2012

Contributor

Yea, it is redundant and install_requires is a stronger requirement since there is no way to turn them off with setup parameters.

@jehiah

jehiah Sep 10, 2012

Owner

The two new items (ujson and host_pool) aren't really install requirements, they are just normal requirements for parts of this library. If pip follows the requirement block I'd like to see these listed as requirements instead of install_requirements.

(also related to my general hate for open source dependency trees, it might be nice to remove ujson completely in favor of conditional simplejson/json imports for standard libraries)

@mynameisfiber

mynameisfiber Sep 10, 2012

Contributor

I can agree to dependency hell... I changed everything but tornado to be requires and I put the ujson imports into try except blocks.

pysimplehttp/src/http.py
+ data = json.dumps(data)
+ return http_fetch(endpoint + '/pub', body=data, method='POST', timeout=1.5)
+
+def nsqd_write(topic, hostpool, data):
@jehiah

jehiah Sep 10, 2012

Owner

some of these functions don't make any sense in this context. Can you trim this file to only what's used by BaseReader

@mynameisfiber

mynameisfiber Sep 10, 2012

Contributor

Consider it trimmed!

pysimplehttp/src/http.py
+ connect_timeout=connect_timeout or timeout,
+ request_timeout=request_timeout or timeout,
+ validate_cert=False,
+ user_agent='bitlybot', **client_options)
@jehiah

jehiah Sep 10, 2012

Owner

please drop this user_agent

@mynameisfiber

mynameisfiber Sep 10, 2012

Contributor

Oops! I completely missed that.

pysimplehttp/src/http.py
+import urllib
+from host_pool import HostPool
+
+from libbitly.formatters import _utf8, _utf8_params
@jehiah

jehiah Sep 10, 2012

Owner

in this context, these functions are not available, and need to be copied here.

@mynameisfiber

mynameisfiber Sep 10, 2012

Contributor

Good catch!

@ploxiln ploxiln referenced this pull request Sep 10, 2012

Closed

Update pysimplehttp #74

Updated pysimplehttp and added BaseReader+deps
This pull request updates the pysimplehttp library and adds the
BaseReader class to help with simplehttp python development.

jehiah added a commit that referenced this pull request Sep 10, 2012

@jehiah jehiah merged commit 4233375 into bitly:master Sep 10, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment