search

michaelhelmick / python-flickr

A Python Library to interface with Flickr REST API, OAuth & JSON Responses
39 stars 14 forks source link

Update up.flickr.com endpoint & always setting "Content-Length" for POST requests #12

Closed morozgrafix closed 10 years ago

morozgrafix commented 10 years ago

Update for upload/replace end points - fixes #10 Added explicit setting of “Content-Length” for POST requests to fix #11

morozgrafix commented 10 years ago

Michael, upload/replace endpoint seems like a trivial change.

The content-length bug is something that I have ran into and notified Flickr service engineers about it. In my opinion Flickr should handle it in a better way, since content-length is NOT required in HTTP/1.1 spec, but should be passed. I didn't see any issues setting it to 0 for all POST requests and then getting it overridden when post contains a file for upload/replace action.

Let me know what you think about this.

Thanks,

Sergey.

P.S. Full disclaimer - I work at Flickr.

michaelhelmick commented 10 years ago

I'm not sure if it will cause some hiccups with requests or not. For that (sorry to bother you, but @Lukasa) -- Do you think always sending Content-Length: 0 will be dangerous for requests (except for the file uploading, where Content-Length will be set

Other than that, the rest looks good. Thanks!

Lukasa commented 10 years ago

That should be fine. =)

Note, however, that if you set Content-Length (to anything) and then set body as an iterator without length (e.g. a generator), we'll do chunked transfer encoding but leave the Content-Length header in place (see kennethreitz/requests#1648). I haven't checked your code to confirm you never do that, but it's worth being aware of.

michaelhelmick commented 10 years ago

@Lukasa We're not iterating to set body anywhere (in this library at least) -- I'll keep that in mind though for a future release that may have an iterator. Thanks!

morozgrafix commented 10 years ago

@michaelhelmick Thanks for accepting the changes. I will follow up once we have an update for Flickr side of things. Will you push this update to pypi for distribution? (I'm not very familiar with how it really works)

Thanks.