[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Giampaolo Rodola' g.rod...@gmail.com added the comment: This is now committed as r81041. I've also removed the long description from what's new file, as you were suggesting. The other two patches should be adapted for 3.2. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Stuart Axon stu.a...@gmail.com added the comment: It would be good for consistency, yes. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Giampaolo Rodola' g.rod...@gmail.com added the comment: A patch including tests and doc changes is in attachment. -- versions: +Python 3.2 -Python 2.7, Python 3.1 Added file: http://bugs.python.org/file17276/ftplib.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Éric Araujo mer...@netwok.org added the comment: Magic methods usually don’t have docstrings, since they implement one operation that is described in one place (think __len__ vs. len). You could remove the doc of __enter__ and turn the doc of __exit__ into a comment or move it to the reST file when you say that FTP supports the context management protocol. What did you add a cmd_noop method that does the same thing as cmd_user for? Will the change from handler to handler_instance not break third-party code? -- nosy: +merwok ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Giampaolo Rodola' g.rod...@gmail.com added the comment: Magic methods usually don’t have docstrings, since they implement one operation that is described in one place (think __len__ vs. len). Agreed, I only left it since it was there in the first place. What did you add a cmd_noop method that does the same thing as cmd_user for? Basically all DummyFTPHandler commands are no-ops except pasv, port, retr, stor and some others. There's no real reason why I added NOOP except for consistency with the FTP procol. What I wanted to do in the new tests was just sending a command and receive a response and the correct way to do that in FTP is using NOOP. Will the change from handler to handler_instance not break third-party code? Not sure what you mean by third party code but it shouldn't break the existing tests if that's what you're worried about. I did that because the original server behavior was to accept only one connection and I needed to connect more than one client in the same test. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Éric Araujo mer...@netwok.org added the comment: Re-reading the patch, I notice it’s in the test module, not on the FTP class, so I guess it’s perfectly fine. Is the string returned by a real FTP server “331 username ok”? (My point is that I think it would be better to have different strings for cmd_noop and cmd_user.) I meant code outside the standard library. I thought this was in ftplib, but since it’s in test_ftplib we don’t have to fear breaking other people’s code. In summary, the diff for ftplib seems good to me, with a minor remark about the docstrings, and I don’t feel confident enough to give a review about the tests. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Giampaolo Rodola' g.rod...@gmail.com added the comment: Is the string returned by a real FTP server “331 username ok”? (My point is that I think it would be better to have different strings for cmd_noop and cmd_user.) Whoops! No that should have been 200 noop ok. I copied cmd_user code and forgot to modify the response string. Thanks. New patch in attachment (I changed the doc a little bit including the with example usage in ftplib.rst other than just in what's new). -- Added file: http://bugs.python.org/file17277/ftplib.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Changes by Giampaolo Rodola' g.rod...@gmail.com: Removed file: http://bugs.python.org/file17276/ftplib.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Éric Araujo mer...@netwok.org added the comment: Ah, I’m glad I had one interesting comment out of three in my first message :) This change, although nice, seems a bit trivial for warranting taking so much place in the what’s new. Or perhaps it’s a good thing; I don’t know if people are aware of the goodness of the with statement far and wide. I’m sure amk will edit the document if he thinks the example is out of place. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Giampaolo Rodola' g.rod...@gmail.com added the comment: Is this is still desirable? If so I can work on a patch for ftplib which provides a finer error handling in case of disconnection while sending QUIT and updates tests which should be modified to fit in the newer test suite. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Changes by Giampaolo Rodola' g.rod...@gmail.com: -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Giampaolo Rodola' g.rod...@gmail.com added the comment: Is this still desirable? If so I can work on a patch for ftplib which provides a finer error handling in case of disconnection while sending QUIT and updates tests which should be modified to fit in the newer test suite. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Stuart Axon stu.a...@gmail.com added the comment: zipfile also would make a good target for a contextmanager (as noted here - http://tarekziade.wordpress.com/2009/01/20/python-standard-lib-give-me-more-withs/ ) -- nosy: +stuaxo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Ezio Melotti ezio.melo...@gmail.com added the comment: There's a patch for zipfile in #5511. -- nosy: +ezio.melotti priority: - normal ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Giampaolo Rodola' billiej...@users.sourceforge.net added the comment: As for ftplib patch you should make sure that close() gets called in any case but never more than once. You can use finally to satisfy the first requirement and self.sock is not None to check the second condition: +def __exit__(self, *args): +Context management protocol. + +Will try to quit() if active, then close(). +If not active, a simple close() call is made. + +if self.sock is not None and self.getwelcome() is not None: +try: +self.quit() +except socket.error, e: +# [Errno 61] is Connection refused +# if the error is different, we want to raise it. +if e.errno != 61: +raise +finally: +if self.sock is not None: +self.close() +else: +self.close() -- nosy: +giampaolo.rodola ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Tarek Ziadé ziade.ta...@gmail.com added the comment: I've simplified the code accordinglyhttp://bugs.python.org/issue49...@ok_message=issue%204972%20files%20edited%20ok@template=item about the name of the vars. while I agree with you, I have use those names (errcode, errmsg) because they are used in getreply() so I thought it was more homogeneous. about the return code , i am wondering if it would be good to add in ftplib all the constantes (http://www.ftpplanet.com/ftpresources/ftp_codes.htm) so things are clearer Added file: http://bugs.python.org/file12823/smtplib.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Changes by Tarek Ziadé ziade.ta...@gmail.com: Removed file: http://bugs.python.org/file12811/smtplib.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Tarek Ziadé ziade.ta...@gmail.com added the comment: Feedback from my blog : other places where a context manager could be good to have : - gzip - zipfile - urllib2.urlopen and urllib.request.urlopen ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Antoine Pitrou pit...@free.fr added the comment: Tarek: gzip and bz2 are already done (http://code.python.org/hg/trunk/rev/d9555936ded9). -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Tarek Ziadé ziade.ta...@gmail.com added the comment: Ah great, thanks for telling Antoine, I missed it on gzip. And the test on bz2 makes me see that my tests are not sufficient (calling with on an instance that has already been used and close) ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Changes by Tarek Ziadé ziade.ta...@gmail.com: Added file: http://bugs.python.org/file12812/imaplib.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Alexander Belopolsky belopol...@users.sourceforge.net added the comment: Please remove the old smtplib.patch: it is confusing to have two attachments with the same name. It will still be available in the history, so nothing will be lost. A nit-pick: 221 is a success code (in smtp 2xx codes are successes and 5xx are errors IIRC), so vars should be simply 'code' (or 'rc' for return code) and 'msg', not 'errcode' and 'errmsg'. Your new code may leave socket open in the case when self.docmd(quit) raises an exception other than SMTPServerDisconnected. BTW, I think your code will be simpler if you don't reuse quit() and simply call docmd(quit) and close()directly in an appropriate try/except/finally statement. ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Tarek Ziadé ziade.ta...@gmail.com added the comment: positive feedbacks on python-ideas, so I'll start to write the patches. targets : - smtplib.SMTP - imaplib.IMAP4 - ftplib.FTP first patch : smtplib (will do ftplib and imaplib as well, then propose this enhancement to python-dev) -- keywords: +patch Added file: http://bugs.python.org/file12792/smtplib.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Alexander Belopolsky belopol...@users.sourceforge.net added the comment: What is the rationale for swallowing all socket exceptions except Connection reset by peer in __exit__? In any case, it is better to use errno.ECONNRESET instead of literal 54. Note that SMTP.quit() calls SMTP.close(), so in the normal termination case, close will be called twice. This is not a real problem since SMTP.close() is a noop on a closed SMTP object, but it does not look right. The double call to close() also makes error path harder to analyze. It appears that if a socket error is raised in the first call to close, it gets caught only to be raised again in the second call (assuming a persistent error). -- nosy: +belopolsky ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Tarek Ziadé ziade.ta...@gmail.com added the comment: What is the rationale for swallowing all socket exceptions except Connection reset by peer in __exit__? I am catching just the error that raises if the connection is closed when calling quit() In any case, it is better to use errno.ECONNRESET instead of literal 54. Right Note that SMTP.quit() calls SMTP.close(), so in the normal termination case, close will be called twice. This is not a real problem since SMTP.close() is a noop on a closed SMTP object, but it does not look right. Right i'll fix that Thanks for teh feedback The double call to close() also makes error path harder to analyze. It appears that if a socket error is raised in the first call to close, it gets caught only to be raised again in the second call (assuming a persistent error). ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
Changes by Tarek Ziadé ziade.ta...@gmail.com: Added file: http://bugs.python.org/file12799/ftplib.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4972 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4972] let's equip ftplib.FTP with __enter__ and __exit__
New submission from Tarek Ziadé ziade.ta...@gmail.com:
In a program, I naturally wrote:
from ftplib import FTP
with FTP('ftp.somewhere.com') as ftp:
... ftp.login('someone', 'pass')
... ...
Until I realized this class is not equipped with __enter__ and __exit__,
I think it could be a simple and pleasant enhancement to add it.
--
components: Library (Lib)
messages: 80026
nosy: tarek
severity: normal
status: open
title: let's equip ftplib.FTP with __enter__ and __exit__
type: feature request
versions: Python 2.7, Python 3.1
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue4972
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
