[Twisted-Python] incompatible change - need revert before release
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
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:
Tom Prince discovered a regression on 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
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
-- Adi Roiban
On Nov 20, 2015, at 12:08 AM, Adi Roiban <adi@roiban.ro> wrote:
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
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
On 20 November 2015 at 10:32, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Nov 20, 2015, at 12:08 AM, Adi Roiban <adi@roiban.ro> wrote:
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
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.
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
On Nov 20, 2015, at 1:42 AM, Adi Roiban <adi@roiban.ro> wrote:
On 20 November 2015 at 10:32, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote:
On Nov 20, 2015, at 12:08 AM, Adi Roiban <adi@roiban.ro <mailto:adi@roiban.ro>> wrote:
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
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.
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.
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
On 20 November 2015 at 12:25, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Nov 20, 2015, at 1:42 AM, Adi Roiban <adi@roiban.ro> wrote:
On 20 November 2015 at 10:32, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Nov 20, 2015, at 12:08 AM, Adi Roiban <adi@roiban.ro> wrote:
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
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.
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.
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.
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
Glyph Lefkowitz <glyph@twistedmatrix.com> writes:
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.
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
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