[Twisted-Python] Exception's Implicit Public API, and Python 3

[Amber "Hawkie" Brown]

We just ran into an issue in Autobahn|Python where a log message that tried to use the message attribute an Exception subclass (twisted.internet.error.ProcessTerminated). The message attribute was deprecated in Python 2.6, and removed in Python 3 -- but it still "works" in Python 2.7.

So, this needs the question: Is this breaking Twisted's backwards compat (that .message isn't there on Python 3)? Should we go through and look at our exception subclasses, and look at what makes sense to have a .message attribute (for example, I'd say it makes sense on ProcessTerminated) and then add it? Or is it just a 2/3 change that should be expected during porting? It seems like an interesting edge case (and another example of "Don't use subclassing, kids" ;) ).

The Autobahn ticket (which we're fixing as I write this email) is here https://github.com/tavendo/AutobahnPython/issues/479 .

Regards,

Amber "Hawkie" Brown
GPG: https://keybase.io/hawkowl
***@atleastfornow.net

[Glyph]

Post by Amber "Hawkie" Brown
We just ran into an issue in Autobahn|Python where a log message that tried to use the message attribute an Exception subclass (twisted.internet.error.ProcessTerminated). The message attribute was deprecated in Python 2.6, and removed in Python 3 -- but it still "works" in Python 2.7.
So, this needs the question: Is this breaking Twisted's backwards compat (that .message isn't there on Python 3)? Should we go through and look at our exception subclasses, and look at what makes sense to have a .message attribute (for example, I'd say it makes sense on ProcessTerminated) and then add it? Or is it just a 2/3 change that should be expected during porting? It seems like an interesting edge case (and another example of "Don't use subclassing, kids" ;) ).

It's Python's API which has changed here, so I don't think that it's Twisted's responsibility to replicate this behavior. Subclassing is bad, and Python's changes of public attributes to such a core language-feature level class as Exception is extra bad, but I think that attempting to simultaneously support every version of Python's public API via Twisted would be a mistake.

For a different example, consider the case of the implicit base of "object". We could faithfully replicate the semantics of old-style classes for all of Twisted's old-style classes, but instead, they remain old-style on py2 (for now) and new-style on py3. The "message" attribute seems the same to me, so I think we should say this is a 2/3 change you have to make when porting your own code, and the onus is on Autobahn in this case.

Put differently, Twisted's "the first one's free" policy applies to upgrading Twisted itself, and not to upgrading Python (or any other dependency). If you upgrade Python and you need to update your code for that, Twisted won't create any additional problems but it won't go out of its way to solve the ones you normally have to deal with.

Sound good?

-glyph

Post by Amber "Hawkie" Brown
The Autobahn ticket (which we're fixing as I write this email) is here https://github.com/tavendo/AutobahnPython/issues/479 <https://github.com/tavendo/AutobahnPython/issues/479> .

[Tobias Oberstein]

Post by Glyph
Put differently, Twisted's "the first one's free" policy applies to
upgrading Twisted itself, and /not/ to upgrading Python (or any other
dependency). If you upgrade Python and you need to update your code for
that, Twisted won't create any additional problems but it won't go out
of its way to solve the ones you normally have to deal with.
Sound good?

Sounds good. We're fine.

FWIW, we need to guard for even more cases

.message there
.args there _and_ a tuple _and_ of length > 0

https://github.com/tavendo/AutobahnPython/commit/303d289de4993b5ffa9bf90c6fed71364f3521e7#diff-ced6a7641cc1e78205bd85d77b484c03R111

/Tobias

[Tom Most]

Would str(failure.value) work? It should be stable from 2.x to 3.x,
barring Unicode differences:

https://docs.python.org/2/library/exceptions.html#exceptions.BaseException
https://docs.python.org/3.5/library/exceptions.html#BaseException

(This convention is quite handy when you want to defer formatting of a
fancy exception message. Just pass args to the constructor and override
__str__ to do the expensive formatting.)

---Tom

Post by Tobias Oberstein

Post by Glyph
Put differently, Twisted's "the first one's free" policy applies to
upgrading Twisted itself, and /not/ to upgrading Python (or any other
dependency). If you upgrade Python and you need to update your code for
that, Twisted won't create any additional problems but it won't go out
of its way to solve the ones you normally have to deal with.
Sound good?

Sounds good. We're fine.
FWIW, we need to guard for even more cases
.message there
.args there _and_ a tuple _and_ of length > 0
https://github.com/tavendo/AutobahnPython/commit/303d289de4993b5ffa9bf90c6fed71364f3521e7#diff-ced6a7641cc1e78205bd85d77b484c03R111
/Tobias
_______________________________________________
Twisted-Python mailing list
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

[Adi Roiban]

snip

Post by Glyph
It's Python's API which has changed here, so I don't think that it's
Twisted's responsibility to replicate this behavior. Subclassing is bad,
and Python's changes of public attributes to such a core language-feature
level class as Exception is extra bad, but I think that attempting to
simultaneously support every version of Python's public API via Twisted
would be a mistake.

What harmed is done if the twisted.internet.error.ProcessTerminated
exception has an explicit message attribute?

Thanks!

[Glyph]

Post by Adi Roiban

Post by Glyph
It's Python's API which has changed here, so I don't think that it's
Twisted's responsibility to replicate this behavior. Subclassing is bad,
and Python's changes of public attributes to such a core language-feature
level class as Exception is extra bad, but I think that attempting to
simultaneously support every version of Python's public API via Twisted
would be a mistake.

What harmed is done if the twisted.internet.error.ProcessTerminated
exception has an explicit message attribute?

I don't like the precedent it sets; this is not part of the compatibility contract we provide, and we already spend tons of energy on compatibility :-). Maintaining stuff like this - and like old-style classes - would be a ton of additional work for no real benefit.
`.messageÂŽ and `.argsÂŽ are implementation accidents, not things that anyone should be relying on in a public API. If we wanted to provide some structured information for programs to use from ProcessTerminated, let's actually give it a good API that documents what it means, and not just stick a random string or some tuples on an object.

-glyph