classification
Title: Remove the warning in urllib docs that it doesn't do certificate validate by default.
Type: behavior Stage: resolved
Components: Documentation Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: orsenthil Nosy List: Julian, benjamin.peterson, christian.heimes, clopez, martin.panter, orsenthil
Priority: normal Keywords: patch

Created on 2017-01-06 17:01 by orsenthil, last changed 2017-01-28 07:06 by orsenthil. This issue is now closed.

Files
File name Uploaded Description Edit
remove_warning.patch orsenthil, 2017-01-06 17:01
issue29182_docs_fix.patch orsenthil, 2017-01-24 06:49
issue29182-patch3.diff orsenthil, 2017-01-27 08:22
Messages (8)
msg284836 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2017-01-06 17:01
It started as a discussion in this issue: http://bugs.python.org/issue22417#msg284604

I think, that warning can be removed.  If no one has any objections, I will commit this attached patch.
msg285978 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2017-01-22 01:08
The warning for urllib2.urlopen() was removed in revision 1882157b298a. However, a couple other warnings were converted to “Changed in version 2.7.9” in revision fb83916c3ea1, which seems safer to me.

Removing documentation almost seems like a step backwards. The usual approach for new features is to document the new behaviour, and when it changed. This has also been done for the “context” etc parameters added in a bug fix release. So I suggest to document that the certificate is verified since 2.7.9, but not beforehand.
msg285983 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2017-01-22 03:18
@Martin, that's a sound advice. I agree to it. I'll change it to a note (instead of warning) which mentions about certificate verification since 2.7.9.
msg286145 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2017-01-24 06:49
Here is the updated patch that addresses the comment.
msg286240 - (view) Author: Julian Berman (Julian) * Date: 2017-01-25 10:42
The change note belongs outside the seealso.

I think also that wasn't exactly what Martin had in mind, I think he meant a `.. versionchanged` directive -- and given that this was originally a warning, personally I'd leave the warning but reword it, it's still relevant for pre-2.7.9 users.

E.g. something like:

.. seealso::

   <requests module info>

.. versionchanged:: 2.7.9

    HTTPS certificates now have hostname verification enabled by default.

.. warning::

    Versions prior to 2.7.9 do not safely verify hostnames. It is not recommended to use this module on these versions -- the aformentioned requests module can be used instead.

Or the like.
msg286345 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2017-01-27 08:22
Hi Julian, here is the patch that addresses you comments. Thanks
msg286347 - (view) Author: Julian Berman (Julian) * Date: 2017-01-27 09:30
Cool! If I can nitpick one last time, in the versionchanged block, you have `HTTPS URIs` but in the warning you use `HTTPS Urls` -- probably best to use consistent casing and URL vs URI.

Other than that, lgtm to merge.
msg286403 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2017-01-28 07:06
Thank you for the review, Julian.
Addressed it and the change is committed at changeset b5125e971fdd
History
Date User Action Args
2017-01-28 07:06:58orsenthilsetstatus: open -> closed
resolution: fixed
messages: + msg286403

stage: commit review -> resolved
2017-01-27 09:30:02Juliansetmessages: + msg286347
2017-01-27 08:22:56orsenthilsetfiles: + issue29182-patch3.diff

messages: + msg286345
2017-01-25 10:42:30Juliansetnosy: + Julian
messages: + msg286240
2017-01-24 06:49:32orsenthilsetfiles: + issue29182_docs_fix.patch

messages: + msg286145
2017-01-22 03:18:11orsenthilsetmessages: + msg285983
2017-01-22 01:08:39martin.pantersetnosy: + martin.panter
messages: + msg285978
2017-01-07 05:39:59berker.peksagsetstage: patch review -> commit review
2017-01-06 17:01:57orsenthilcreate