[Twisted-Python] Exception's Implicit Public API, and Python 3
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 hawkowl@atleastfornow.net
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
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> .
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/303d289de4993b5ffa9bf90c6fe... /Tobias
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 On 09/06/2015 01:20 PM, Tobias Oberstein wrote:
On 6 September 2015 at 06:07, Glyph <glyph@twistedmatrix.com> wrote:
On Sep 5, 2015, at 04:03, Amber Hawkie Brown <hawkowl@atleastfornow.net> wrote:
snip
What harmed is done if the twisted.internet.error.ProcessTerminated exception has an explicit message attribute? Thanks! -- Adi Roiban
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
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
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> .
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/303d289de4993b5ffa9bf90c6fe... /Tobias
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 On 09/06/2015 01:20 PM, Tobias Oberstein wrote:
On 6 September 2015 at 06:07, Glyph <glyph@twistedmatrix.com> wrote:
On Sep 5, 2015, at 04:03, Amber Hawkie Brown <hawkowl@atleastfornow.net> wrote:
snip
What harmed is done if the twisted.internet.error.ProcessTerminated exception has an explicit message attribute? Thanks! -- Adi Roiban
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
participants (5)
-
Adi Roiban -
Amber "Hawkie" Brown -
Glyph -
Tobias Oberstein -
Tom Most