[Twisted-Python] incompatible change - need revert before release
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
Tom Prince discovered a regression on https://twistedmatrix.com/trac/ticket/7016#comment:14 <https://twistedmatrix.com/trac/ticket/7016#comment:14> - I think that this was introduced after 15.4, so it needs to be rolled back (or fixed, if someone can get to it before the revert) in 15.5. -glyph
![](https://secure.gravatar.com/avatar/c194a4d2f2f8269aa052942e87985198.jpg?s=120&d=mm&r=g)
It was released before 15.4 Twisted Web 15.2.0 (2015-05-18) =============================== Features -------- - twisted.web.server.Site accepts requestFactory as constructor argument. (#7016) ---------- Not sure if rollback is the right thing to do... but I have no idea how to proceed as any change will back the compatibility. I guess that we should just create a normal bug ticket and fix this issue Regards, Adi On 20 November 2015 at 07:06, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
-- Adi Roiban
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
Thanks for finding the changelog entry; sorry for the false alarm. If it's been in a release, then there's probably nothing to do. It's a shame that this went out, but once a breakage like this has happened we have to live with it because otherwise, as you say, we'd be breaking compatibility for the people that already upgraded. For those that need to support both versions, keyword arguments are the way to go. -glyph
![](https://secure.gravatar.com/avatar/c194a4d2f2f8269aa052942e87985198.jpg?s=120&d=mm&r=g)
On 20 November 2015 at 10:32, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
Well, in public interfaces we could just stop mixing *args and **kwargs with other arguments. It is more work for maintainers, but as a library user I find it much easier to see the exact args in the docs, rather than seeing *args / **kwargs and then navigating the inheritance path to find out all supported arguments. -- Adi Roiban
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
I'm not quite sure what you're referring to in this case; but generally, I agree. If you pass a parameter, you should document it with @param even if your arg list says *args / **kwargs. This is how I tried to document, for example, optionsForClientTLS. -glyph
![](https://secure.gravatar.com/avatar/c194a4d2f2f8269aa052942e87985198.jpg?s=120&d=mm&r=g)
On 20 November 2015 at 12:25, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
Instead of def __init__(self, resource, requestFactory=None, *args, **kwargs): http.HTTPFactory.__init__(self, *args, **kwargs) you can have def __init__(self, resource, logFile=None, requestFactory=None): http.HTTPFactory.__init__(self, logFile=logFile) Duplicating documentation is ugly... maybe we can "improve" pydoctor to support something like this. Like @see but instead of creating a link, the code is duplicated... but maybe a link is enough def __init__(self, resource, logFile=None, requestFactory=None): """" Some description. @include http.HTTPFactory.__init__.logFile """" http.HTTPFactory.__init__(self, logFile=logFile) -- Adi Roiban
![](https://secure.gravatar.com/avatar/e81edff3af564b86f4c9d780a8023299.jpg?s=120&d=mm&r=g)
Glyph Lefkowitz <glyph@twistedmatrix.com> writes:
There are perhaps a couple of things we can do. The types of the arguments should usually (always?) be different, so we could at least warn if we suspect the wrong thing was passed, if not either error out or do the right thing, so code won't silently or inexcplicably fail later. We could also deprecate passing an argument as a positional argument, so eventually upgrading will always get an error, rather than incorrect behavior. Tom
![](https://secure.gravatar.com/avatar/e81edff3af564b86f4c9d780a8023299.jpg?s=120&d=mm&r=g)
Glyph Lefkowitz <glyph@twistedmatrix.com> writes:
Tom Prince discovered a regression on https://twistedmatrix.com/trac/ticket/7016#comment:14 <https://twistedmatrix.com/trac/ticket/7016#comment:14> - I think that this was introduced after 15.4, so it needs to be rolled back (or fixed, if someone can get to it before the revert) in 15.5.
Sadly, this has already been released (in 15.2), and changing it back would also be an incompatible change. And, to set the record straight, the issue was reported by sveinse on #twisted. Tom
![](https://secure.gravatar.com/avatar/c194a4d2f2f8269aa052942e87985198.jpg?s=120&d=mm&r=g)
It was released before 15.4 Twisted Web 15.2.0 (2015-05-18) =============================== Features -------- - twisted.web.server.Site accepts requestFactory as constructor argument. (#7016) ---------- Not sure if rollback is the right thing to do... but I have no idea how to proceed as any change will back the compatibility. I guess that we should just create a normal bug ticket and fix this issue Regards, Adi On 20 November 2015 at 07:06, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
-- Adi Roiban
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
Thanks for finding the changelog entry; sorry for the false alarm. If it's been in a release, then there's probably nothing to do. It's a shame that this went out, but once a breakage like this has happened we have to live with it because otherwise, as you say, we'd be breaking compatibility for the people that already upgraded. For those that need to support both versions, keyword arguments are the way to go. -glyph
![](https://secure.gravatar.com/avatar/c194a4d2f2f8269aa052942e87985198.jpg?s=120&d=mm&r=g)
On 20 November 2015 at 10:32, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
Well, in public interfaces we could just stop mixing *args and **kwargs with other arguments. It is more work for maintainers, but as a library user I find it much easier to see the exact args in the docs, rather than seeing *args / **kwargs and then navigating the inheritance path to find out all supported arguments. -- Adi Roiban
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
I'm not quite sure what you're referring to in this case; but generally, I agree. If you pass a parameter, you should document it with @param even if your arg list says *args / **kwargs. This is how I tried to document, for example, optionsForClientTLS. -glyph
![](https://secure.gravatar.com/avatar/c194a4d2f2f8269aa052942e87985198.jpg?s=120&d=mm&r=g)
On 20 November 2015 at 12:25, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
Instead of def __init__(self, resource, requestFactory=None, *args, **kwargs): http.HTTPFactory.__init__(self, *args, **kwargs) you can have def __init__(self, resource, logFile=None, requestFactory=None): http.HTTPFactory.__init__(self, logFile=logFile) Duplicating documentation is ugly... maybe we can "improve" pydoctor to support something like this. Like @see but instead of creating a link, the code is duplicated... but maybe a link is enough def __init__(self, resource, logFile=None, requestFactory=None): """" Some description. @include http.HTTPFactory.__init__.logFile """" http.HTTPFactory.__init__(self, logFile=logFile) -- Adi Roiban
![](https://secure.gravatar.com/avatar/e81edff3af564b86f4c9d780a8023299.jpg?s=120&d=mm&r=g)
Glyph Lefkowitz <glyph@twistedmatrix.com> writes:
There are perhaps a couple of things we can do. The types of the arguments should usually (always?) be different, so we could at least warn if we suspect the wrong thing was passed, if not either error out or do the right thing, so code won't silently or inexcplicably fail later. We could also deprecate passing an argument as a positional argument, so eventually upgrading will always get an error, rather than incorrect behavior. Tom
![](https://secure.gravatar.com/avatar/e81edff3af564b86f4c9d780a8023299.jpg?s=120&d=mm&r=g)
Glyph Lefkowitz <glyph@twistedmatrix.com> writes:
Tom Prince discovered a regression on https://twistedmatrix.com/trac/ticket/7016#comment:14 <https://twistedmatrix.com/trac/ticket/7016#comment:14> - I think that this was introduced after 15.4, so it needs to be rolled back (or fixed, if someone can get to it before the revert) in 15.5.
Sadly, this has already been released (in 15.2), and changing it back would also be an incompatible change. And, to set the record straight, the issue was reported by sveinse on #twisted. Tom
participants (3)
-
Adi Roiban
-
Glyph Lefkowitz
-
Tom Prince