Re: [Twisted-Python] Backward incompatible fixes to backward incompatible changes

[Glyph]

On Thu, Apr 25, 2019, at 9:52 AM, Jean-Paul Calderone wrote:
> Hello all,
> 
> I'd like to discuss the project policy/process for dealing with incorrect 
> backward incompatibilities that end up released.
> 
> As a case study, we could look at https://twistedmatrix.com/trac/ticket/9410 
>  / 
> https://github.com/twisted/twisted/pull/1106 
> 
> 
> In Twisted 16.3.0 the behavior of Request.write was changed so that it raises 
> an AttributeError if called after the connection has closed.  Prior to that 
> release, it silently did nothing in that case.
> 
> Now, three years later, we have a PR which proposed restoring the behavior of 
> silently ignoring the write to a closed connection.
> 
> The original change was incompatible.  The new change is incompatible.  What 
> should win out?

There are two things to address here, I think:

In the general case, can we reverse changes to behavior which were desirable, 
but by-policy incompatible, and did not go through the incompatibility 
exception process, without going through the incompatibility exception process?

I'm choosing my words carefully here; changes might be "desirable" even if 
they're not intentional; they might be something that the caller expects even 
if they're not explicitly documented.  In general I'd say that we have to be 
careful with this kind of reversal and we should probably go through the compat 
exception process.  But I think the change in question doesn't actually fall 
into this category, because it's an exception that nobody wanted, wasn't 
documented, and wasn't intentional.  So it's in this other category:

Should we consider the raising of an undocumented exception to be a 
compatibility surface we must maintain compatibility with?

In this case, I think it depends on the type of the exception - specifically, 
whether the exception is defined in Twisted itself, or defined in a library 
that is clearly being invoked and whose behavior should be propagated.  So, for 
example, if you call connection.foo().bar() and you get FooCannotBeBarredError, 
this might something to be considered a compatibility concern.  However, if you 
call connection.foo().bar() and get an undocumented stdlib exception 
(KeyError, AttributeError, ValueError), I think it's okay to implicitly treat 
these as "gross violations of specifications".

In the specific case you dealt with I agree with your evaluation and I'm glad 
we've landed the bugfix :-).

-g___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] Backward incompatible fixes to backward incompatible changes

[Evilham]

Jean-Paul Calderone  writes:


On Thu, Apr 25, 2019 at 12:52 PM Jean-Paul Calderone <
exar...@twistedmatrix.com> wrote:


Hello all,

I'd like to discuss the project policy/process for dealing with 
incorrect

backward incompatibilities that end up released.



Considering the lack of interest in the topic, I'm going to 
suppose that
there is no policy and the decision is made case-by-case.  In 
the case I
referenced, I'm going to approve the change in behavior as 
"compatible" by

on the grounds that it restores the original behavior.



What you say makes sense and, as read on one of the tickets, if 
there *is* a need for that situation to raise an error: 
`AttributeError` certainly isn't the kind of error.

So that should be a totally different discussion.

As for a general policy: it doesn't look like this situation 
happens often enough for there to be a need for one (de facto, 
indeed that means that it is made on a case-by-case basis).
Effort in general should be to prevent the situation from 
happening, which everyone regularly involved in Twisted does a 
great job at already.


Basically: ACK, fully agreed, you are not talking to the void.


As a case study, we could look at
https://twistedmatrix.com/trac/ticket/9410
 /
https://github.com/twisted/twisted/pull/1106

In Twisted 16.3.0 the behavior of Request.write was changed so 
that it
raises an AttributeError if called after the connection has 
closed.  Prior

to that release, it silently did nothing in that case.

Now, three years later, we have a PR which proposed restoring 
the behavior

of silently ignoring the write to a closed connection.

The original change was incompatible.  The new change is 
incompatible.

What should win out?


--
Evilham

___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] Backward incompatible fixes to backward incompatible changes

[Jean-Paul Calderone]

On Thu, Apr 25, 2019 at 12:52 PM Jean-Paul Calderone <
exar...@twistedmatrix.com> wrote:

> Hello all,
>
> I'd like to discuss the project policy/process for dealing with incorrect
> backward incompatibilities that end up released.
>

Considering the lack of interest in the topic, I'm going to suppose that
there is no policy and the decision is made case-by-case.  In the case I
referenced, I'm going to approve the change in behavior as "compatible" by
on the grounds that it restores the original behavior.

Jean-Paul


> As a case study, we could look at
> https://twistedmatrix.com/trac/ticket/9410
>  /
> https://github.com/twisted/twisted/pull/1106
>
> In Twisted 16.3.0 the behavior of Request.write was changed so that it
> raises an AttributeError if called after the connection has closed.  Prior
> to that release, it silently did nothing in that case.
>
> Now, three years later, we have a PR which proposed restoring the behavior
> of silently ignoring the write to a closed connection.
>
> The original change was incompatible.  The new change is incompatible.
> What should win out?
>
> Jean-Paul
>
>
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

[Twisted-Python] Backward incompatible fixes to backward incompatible changes

[Jean-Paul Calderone]

Hello all,

I'd like to discuss the project policy/process for dealing with incorrect
backward incompatibilities that end up released.

As a case study, we could look at https://twistedmatrix.com/trac/ticket/9410
 /
https://github.com/twisted/twisted/pull/1106

In Twisted 16.3.0 the behavior of Request.write was changed so that it
raises an AttributeError if called after the connection has closed.  Prior
to that release, it silently did nothing in that case.

Now, three years later, we have a PR which proposed restoring the behavior
of silently ignoring the write to a closed connection.

The original change was incompatible.  The new change is incompatible.
What should win out?

Jean-Paul
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python