homepage

Executive summary of the patch:
The attached patch removes the use of __private attributes in the smtpd
module allowing it to be extensible without needing to use the
"_<classname>__<attributename>" hack.
Summary of the patch's changes:
1. removes the unused __conn and __addr attributes
2. renames __server to smtp_server
3. renames __lines to received_lines
4. renames __state to smtp_state
5. renames __greeting to seen_greeting, and alters the default to empty
string to match the anticipated data
6. renames __mailfrom to mailfrom
7. renames __date to received_data
8. renames __fqdn to fqdn
9. removes __peer and uses base class' addr attribute
The existing unit tests contained within test_smtplib pass. Additional
tests could be written if it's deemed necessary.
There is a chance this patch will break backward compatibility with
programs that use the private-variable-access hack. A more complex patch
could be written providing greater compatibility if it's deemed necessary.

Would this patch be acceptable, yes or no?

The patch as-is can't be accepted if not for Python 4.x maybe, obviously because it's just too breaking.
A proper patch would provide aliases for the removed attributes and raise a DeprecationWarning in case they are accessed.
It would be suitable for the the next major release (3.2 at the current time) and the total removal of the deprecated attributes would happen only in 3.3 if not later.
Also, I see no reason in turning public attributes like self.__line, self.__state and self.__greeting.
They should just be private (one underscore) and their name kept the same (e.g. no __server -> smtp_server, __greeting -> seen_greting, etc...).
Finally, tests should definitively be written.

Giampaolo,
I think I can see where you're coming from: assuming that someone else must have also had to resort to the name-mangling hack to extend the class? In that case yes, my patch would break their code. I'll look at re-working it to use properties while retaining the underlying attributes. Would that be acceptable?
What additional tests would you deem necessary?

> Would that be acceptable?
I guess it would. Deciding to use that naming convention was a bad design choice in the first place, hence your proposal is perfectly reasonable, in my opinion.
> What additional tests would you deem necessary?
I think that a test which iterates over all renamed attributes and makes sure that DeprecationWarning is raised would be fine.
Also, "__attrname" and "attrname" should be checked for equality.

> The patch as-is can't be accepted if not for Python 4.x maybe,
> obviously because it's just too breaking.
With all due respect, this sounds a bit silly. If the attributes were of the "__private" kind, they weren't meant to be used by other classes, and so there's no problem in making them public. (the reverse would be more annoying)
However, making them public means they should be maintained with the same semantics in future versions, which might be too much of a burden. In this case, perhaps you want to make them of the "_private" kind instead.

> If the attributes were of the "__private" kind, they weren't meant to 
> be used by other classes, and so there's no problem in making them 
> public.
Generally I would agree with you but this case is different, imho.
The problem here is that those items didn't have to be declared "__private" in the first place. As a consequence of this I think it's reasonable to expect that during the years some code out there might got to rely on them by using name-mangling hack.
After all smtpd module is very old and SMTPHandler class likely to be subclassed because of its basic implementation, and that is why I see some similarities with issue 8483.
I was of the opinion that just removing the __getattr___ ugliness without passing through the usual DeprecationWarning period supposed to be applied in such circumstances would cause no harm, until I realized I was relying on that very functionality in pyftpdlib and I didn't even know it.

After discussing with core devs at the EuroPython sprint I will implement a different approach: new attributes with the old, private attributes implemented as properties over the new attributes. The properties responsible for this will raise PendingDeprecationWarnings.
I'll also be improving (well, *implementing*) test coverage for the module while I'm at it.

Committed in revision 83125.