Guess content type

Guess content type for posting files, just like what urllib3 does.


Thanks for this!

In principle this looks fine. My only worry is that technically this changes what our multipart requests look like on the wire, and I worry about breaking currently functional code. I'm interested to see if @sigmavirus24 is as worried about that as me.


@Lukasa 

> My only worry is that technically this changes what our multipart requests look like on the wire

I don't think it changed anything. A proper guess of file content type shouldn't break anything (I hope).

ping @sigmavirus24 for a code review.


I mean, in the literal sense it definitely changed something.

This:

```
Content-Disposition: form-data; name="file"; filename="name"
```

 is not equal to this:

```
Content-Disposition: form-data; name="file"; filename="name"
Content-Type: text/plain
```

You're right, this _shouldn't_ break things, but this is the web we're talking about, it definitely has a chance to. =) That's why I want @sigmavirus24 to take a look.


@Lukasa Yes, you are definitely right. But it has a chance to do such thing:

```
{'file': (filename, data, None)}
```

I think we are here to discuss which is the best default way to handle file uploading.


I'm totally in agreement, and you'll note that right now I'm sitting on the fence until another of the core developers joins the discussion. =) However, in the interest of openness, I should note that my default position would be to _not_ change things unless necessary. I'm prepared to be swayed, but right now I'm -0.


@lepture are you suggesting that in order to achieve the old behaviour (that probably most of our users rely on) they'll have to pass a tuple of `(filename, data, None)`? To me that is entirely backwards incompatible behaviour and would indicate this change would need to wait until 3.0.0.

I share @Lukasa's concern that while this is technically valid and should not break anything that it might. I'm trying to imagine a file type for which `mimetools` could return the wrong `Content-Type` header. All I can imagine is something entirely proprietary that isn't necessary publicly available and that a corporate client is posting to a service they own. It's entirely plausible for them to not use a `Content-Type` header in that case because their server will know how to handle it. In that case, since they control both ends, I can't imagine this would necessarily break anything but it would certainly cause them a headache.

That aside, my larger sticking point is that this is a backwards incompatible change. To achieve the same behaviour as before, users have to pass extra parameters that they didn't before. That's not good.


@sigmavirus24 I am not so sure now. I think it would be better for a smart guessing of content type. Because when I first use this lib for posting files, I thought it should handle smarter.

I had a look at the code. And I found that urllib3 handles smart, but requests not. I thought maybe requests just missed it. I didn't expect that you intended to handle it this way.

However, if you find this patch is meaningless, just ignore it.


@lepture like I said, it _should not_ break anything and I'm sure it will improve some user's experience with requests. The issue is that it is not backwards compatible because to keep the same behaviour a user has to pass extra parameters. The key part is that I **never** said this pull request was "meaningless". I think @Lukasa and I both agree that it is an improvement, but it is an improvement we cannot make until we start considering a 3.0.0 release which seems to me to be very far off.


Mm, 3.0 is a very long way away indeed. I'm trying to work out how we can keep track of this idea without having Kenneth accidentally merge it. Maybe raise an issue to keep track of "things we'd like in a hypothetical 3.0 release"?


The pre-3.0 solution of course would be to use the toolbelt, but I still haven't released that yet.


This seems like a lovely feature to me, and when I think of backwards compatibility, I mostly consider API changes (of which this isn't). 

I can go either way on this. 


@shazow how accurate is this detection?


@kennethreitz As accurate as Python's stdlib mimetype guessing. http://docs.python.org/2/library/mimetypes.html#mimetypes.guess_type


It does get some stuff laughably wrong: for instance, files whose filenames end in `.jpg` or `.png` get detected as `image/pjpeg` and `image/x-png` on Windows.


@Lukasa Time for another Python core patch? :P


[There already is one.](http://bugs.python.org/issue15207). =)


The summary of that issue, for those who don't want to wade through it:

`guess_type` is really stupid on Windows on versions of Python before 2.7.6/3.3.3rc1, and gets a number of quite common types wrong. This situation makes me officially -1 on this patch _until_ we drop support for Python 2.6. Once that happens we can say that modern versions of Python will do the right thing on all platforms.


@Lukasa That's weird. I never knew it would be a disaster on Windows.


@lepture No worries, I'd have been amazed if you did know. I only happen to know because I bumped into this totally by accident [when working on urllib3](https://github.com/shazow/urllib3/issues/256#issuecomment-26619884).


I'm with @Lukasa until we can drop 2.6, let's leave this feature out. A point of inquiry though - hasn't 2.6 seen the last of it's bug/security releases? In other words, I think 2.6 has reached its end of life. We could start planning 3.0 to abandon 2.6 and introduce this feature.


@sigmavirus24 Is python 2.7 the default python on every linux distribution now? If so, I think 2.6 has reached its end of life.


2.6 _has_ stopped receiving security releases, but sadly we can't drop support for it. This is for three reasons. Firstly, the more conservative OSes still ship with it (hello RHEL and CentOS!). Secondly, even if they start shipping with 2.7, they'll still have LTS releases that have 2.6. Finally, now that we're vendored into `pip` we need to do our best to support what `pip` supports to avoid leaving ourselves between a rock and a hard place. Sadly, that includes 2.6.


Actually, that's a point. @dstufft, can you give us an idea of what `pip`s release cycle looks like from the perspective of supporting 2.6?


That isn't the definition of EOL and the answer to that question is no. RHEL still ships with a version prior to 2.7. Others do too but I just woke up


Historically we've dropped support for a language whenever someone suggested we should and the rest of the core team agreed on it. Actually quite recently we've started work on putting all download events on PyPI into a SQL database so it can be queried, here's the results of that:

```
SELECT substring(python_version for 3), COUNT(*) FROM downloads WHERE package_name = 'pip' GROUP BY substring(python_version for 3) ORDER BY COUNT(*) DESC;

 substring | count
-----------+--------
 2.7       | 177841
           | 109074
 2.6       |  73848
 3.3       |   9736
 3.2       |   1840
 2.5       |    921
 1.1       |    569
 2.4       |    278
 3.4       |     81
 3.1       |     42
 3.0       |      1
 2.1       |      1
(12 rows)
```

And because this is requests!

```
SELECT substring(python_version for 3), COUNT(*) FROM downloads WHERE package_name = 'requests' GROUP BY substring(python_version for 3) ORDER BY COUNT(*) DESC;

 substring | count
-----------+--------
 2.7       | 377312
 2.6       |  53098
 3.3       |   9057
           |   3798
 3.2       |   3044
 1.1       |    314
 2.5       |     87
 3.4       |     69
 2.4       |     62
 3.1       |     11
 3.0       |      1
(11 rows)
```

The above is starting on 1/2 and through 1/15. I can give different information if it's wanted FWIW but I doubt pip is going to be willing to drop 2.6 with numbers like that being put up.


Agreed. So the consensus is that we need to support 2.6 for the foreseeable future. This means we'll close this issue rather than have it sit around indefinitely. Thanks for your contributions everyone!


Lol, I never would have let you guys do this. Nice try though :P

## 

Kenneth Reitz

> On Jan 16, 2014, at 4:53 AM, Cory Benfield notifications@github.com wrote:
> 
> Closed #1861.
> 
> —
> Reply to this email directly or view it on GitHub.