<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<title>Re: [Python-Dev] Asking for feedback about fixing `ftplib'
(issue25458)</title>
On 28.12.2016 21:02, Giampaolo Rodola' wrote:<br>
<blockquote
cite="mid:CAFYqXL9V5TZtXDFELVMUFSj3wU5eW0Zvqn69wu3LUN3fQMZFpQ@mail.gmail.com"
type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Dec 28, 2016 at 3:57 PM, Ivan
Pozdeev <span dir="ltr"><<a
href="mailto:vano@mail.mipt.ru" target="_blank"
moz-do-not-send="true">vano@mail.mipt.ru</a>></span>
wrote:<br>
<blockquote style="BORDER-LEFT: rgb(204,204,204) 1px solid;
MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex"
class="gmail_quote">
<div bgcolor="#FFFFFF"><span class="gmail-">On 25.12.2016
17:21, Giampaolo Rodola' wrote:<br>
<blockquote type="cite">
<div dir="ltr">From what I understand the problem
should be fixed in urllib so that it always closes
the FTP connection. I understand this is what
happens in recent 3.x versions but not on 2.7.</div>
</blockquote>
</span>urllib in 2.7 should indeed be fixed to close FTP
connections rather than leave them dangling, no question
about that.<br>
</div>
</blockquote>
<div><br>
</div>
<div>To me it looks like this is the only issue (currently
tracked in <a href="http://bugs.python.org/issue28931"
moz-do-not-send="true">http://bugs.python.org/issue28931</a>).</div>
</div>
</div>
</div>
</blockquote>
So, closing the control connection ungracefully and without checking
the response is a non-issue? The right way to do it?<br>
<blockquote
cite="mid:CAFYqXL9V5TZtXDFELVMUFSj3wU5eW0Zvqn69wu3LUN3fQMZFpQ@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> <br>
</div>
<blockquote style="BORDER-LEFT: rgb(204,204,204) 1px solid;
MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex"
class="gmail_quote">
<div bgcolor="#FFFFFF">I tried to fix urllib so that it's
guaranteed to call voidresp(), and failed. Too
complicated, effectively reimplementing a sizable part
of FTP client logic is required, perhaps even
monkey-patching ftplib to be able to set flags in the
right places.</div>
</blockquote>
<div><br>
</div>
<div>Why did you fail? Why urllib can't close() ->
voidresp() the FTP session once it's done with it? <br>
</div>
</div>
</div>
</div>
</blockquote>
A simple voidresp() in a socket close handler doesn't work here.
That's why I failed.<br>
* Sometimes, the error needs to be ignored, and sometimes, it
doesn't. With the current architecture, I cannot check which is the
case.<br>
* Error checking can't be done in a close handler. Close handlers
(including this one) are called in finally blocks, raising an
exception here would disrupt further cleanup actions, leaving
objects in an undefined state, and mask the current exception if
there's any.<br>
<br>
What is more important, the entire urllib.ftpwrapper rubs me the
wrong way.<br>
urllib's use case is pretty standard. All it does is the basic FTP
premise of log in, retrieve, log out. So, logically speaking, it
shouldn't have to implement loads upon loads of custom logic on top
of ftplib if ftplib is to be called a production-ready library. Any
logic it has to implement shall be strictly application-specific. <br>
<br>
Since it has to, there can only be two possible conclusions: either
it uses it not in the way intended, or ftplib is an incomplete
library and is missing some critical parts that are required for
practical use.<br>
<br>
<br>
I won't comment on anything further on that boils down to these two
points.<br>
As long as I fail to bring them across, any further discussion is
moot.<br>
<blockquote
cite="mid:CAFYqXL9V5TZtXDFELVMUFSj3wU5eW0Zvqn69wu3LUN3fQMZFpQ@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote style="BORDER-LEFT: rgb(204,204,204) 1px solid;
MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex"
class="gmail_quote">
<div bgcolor="#FFFFFF">With simple commands (<code
class="gmail-m_-1450766179390561399descname">voidcmd</code>)
and self-contained transfer commands
(retrlines/retrbinary), ftplib does incapsulate
statefulness, by handling them atomically. But with
transfercmd(), it returns control to the user halfway
through.<br>
</div>
</blockquote>
<div><br>
</div>
<div>That's by design. ftplib's transfercmd() just works
like that: it's a low level method returning a "data"
socket and it's just up to you to cleanly close the
session (close() -> voidresp()) once you're done with
it. Most of the times you don't even want to use
transfercmd() in the first place, as you just use stor*
and retr* methods.</div>
</div>
</div>
</div>
</blockquote>
<blockquote
cite="mid:CAFYqXL9V5TZtXDFELVMUFSj3wU5eW0Zvqn69wu3LUN3fQMZFpQ@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote style="BORDER-LEFT: rgb(204,204,204) 1px solid;
MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex"
class="gmail_quote">
<div bgcolor="#FFFFFF">At this point, if ftplib doesn't
itself keep track that the control connection is
currently in invalid state for the next command, the
user is forced to do that themselves instead. </div>
</blockquote>
<div><br>
</div>
<div>Hence why I suggested a docfix. <br>
</div>
</div>
</div>
</div>
</blockquote>
<blockquote
cite="mid:CAFYqXL9V5TZtXDFELVMUFSj3wU5eW0Zvqn69wu3LUN3fQMZFpQ@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote style="BORDER-LEFT: rgb(204,204,204) 1px solid;
MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex"
class="gmail_quote">
<div bgcolor="#FFFFFF">And guess what - urllib has to use
ftplib through a wrapper class that does exactly that.
That's definitely a "stock logic part" 'cuz it's an
integral part of FTP rather than something specific to
user logic.<br>
</div>
</blockquote>
<div><br>
</div>
<div>That wrapper should just cleanly close the session. </div>
<div> </div>
<blockquote style="BORDER-LEFT: rgb(204,204,204) 1px solid;
MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex"
class="gmail_quote">
<div bgcolor="#FFFFFF">The other "stock logic part"
currently missing is error handling during transfer. If
the error's cause is local (like a local exception that
results in closing the data socket prematurely, or the
user closing it by hand, or an ABOR they sent midway),
the error code from the end-of-transfer response should
be ignored, otherwise, it shouldn't.<br>
</div>
</blockquote>
<div><br>
</div>
<div>
<div>Absolutely not. Base ftplib should NOT take any
deliberate decision such as ignoring an error.</div>
</div>
</div>
</div>
</div>
</blockquote>
<blockquote
cite="mid:CAFYqXL9V5TZtXDFELVMUFSj3wU5eW0Zvqn69wu3LUN3fQMZFpQ@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div>
<div>I can even come up with use cases: use ftplib to test
FTP servers so that they *do* reply with an error code
in certain conditions. Here's a couple examples in
pyftpdlib:</div>
<div><br>
</div>
<div><a
href="https://github.com/giampaolo/pyftpdlib/blob/17bebe3714752b01e43ab60d2771308f4594bd99/pyftpdlib/test/test_functional.py#L1354-L1369"
moz-do-not-send="true">https://github.com/giampaolo/pyftpdlib/blob/17bebe3714752b01e43ab60d2771308f4594bd99/pyftpdlib/test/test_functional.py#L1354-L1369</a></div>
</div>
<div><br>
</div>
<div><a
href="https://github.com/giampaolo/pyftpdlib/blob/17bebe3714752b01e43ab60d2771308f4594bd99/pyftpdlib/test/test_functional.py#L1973-L1980"
moz-do-not-send="true">https://github.com/giampaolo/pyftpdlib/blob/17bebe3714752b01e43ab60d2771308f4594bd99/pyftpdlib/test/test_functional.py#L1973-L1980</a></div>
<div> <br>
</div>
</div>
</div>
</div>
</blockquote>
If you claim that testing a server is an intended use case for
ftplib (rather than meddling in its internals at your own risk), you
need to document all the other low-level functions that you use
(like putcmd) for your claim to stand. The currently published
interface doesn't offer a sufficiently fine-grained control for that
use case. Its line-of-support is "elementary compounds", none of
which leaves the control connection in an invalid state.<br>
<blockquote
cite="mid:CAFYqXL9V5TZtXDFELVMUFSj3wU5eW0Zvqn69wu3LUN3fQMZFpQ@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote style="BORDER-LEFT: rgb(204,204,204) 1px solid;
MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex"
class="gmail_quote">
<div bgcolor="#FFFFFF">Other than making ftplib keep track
of the session's internal state while the user has
control and providing the custom error handling logic to
them, another solution is to never release control to
the user midway, i.e. ban transfercmd() entirely and
only provide self-contained retr*()/dir()/whatever, but
let the callback signal them to abort the transfer. That
way, the state would be kept implicitly - in the
execution pointer.</div>
</blockquote>
<div><br>
</div>
<div>Banning transfercmd() means renaming it to
_transfercmd() which is a no-no for backward
compatibility. Also, as I've shown above, transfercmd()
does have a use case which relies on it behaving like that
(return control midway). <br>
</div>
</div>
</div>
</div>
</blockquote>
Whyever would a rename be required? More than a few ftplib.FTP's
members are undocumented outright and they don't start with an
underscore.<br>
Anyway, I only mentioned this as one of the possible lines of
action.<br>
<blockquote
cite="mid:CAFYqXL9V5TZtXDFELVMUFSj3wU5eW0Zvqn69wu3LUN3fQMZFpQ@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div><br>
</div>
-- <br>
<div class="gmail_signature">
<div dir="ltr">
<div>Giampaolo - <a href="http://grodola.blogspot.com"
target="_blank" moz-do-not-send="true">http://grodola.blogspot.com</a></div>
<div><br>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
<pre class="moz-signature" cols="72">--
Regards,
Ivan</pre>
</body>
</html>