[Twisted-Python] Twisted v21.2.0 breaks Crossbar.io

[Jean-Paul Calderone]

On Wed, Mar 3, 2021 at 3:08 AM Glyph <glyph at twistedmatrix.com> wrote:

>
> On Mar 2, 2021, at 6:10 PM, Jean-Paul Calderone <exarkun at twistedmatrix.com>
> wrote:
>
> Policy aside, this change doesn't seem like much of an improvement to me.
> If I were to guess, I would guess the change was made to satisfy some check
> Mypy is now being asked to make about Twisted.  If that's the case, it
> seems unfortunate that real-world software is suffering so that a synthetic
> goal can be achieved.  I do recognize there is a perception that practical
> value can come from attending to the errors Mypy reports.  It would
> probably benefit everyone if more care were taken to consider the
> real-world consequences of changes that are made to satisfy the
> non-real-world goalposts set by tools like Mypy.
>
>
> I do think that Mypy was likely highlighting a real issue here, and it
> should have been fixed.  We had documented interfaces already, and we were
> failing to adhere to them properly, and Mypy was complaining about that.
>
> For easy reference, the change that made these fixes is here
> https://github.com/twisted/twisted/pull/1298
>
> This genre of fix is definitely something we should unwind over time,
> although it does make it easier to start mypy-clean rather than have
> hundreds of exclusion lines that need to be manually adjusted, so on
> balance I agree with this tradeoff at the point it was made.
>

Broadly, I agree.  But not with this part.  It seems like there is clearly
a trade-off that is better for everyone.  The trade-off represented by
#1298:

   - Breaks application code without providing any new functionality or
   fixing any bugs
   - Captures a long list of TODOs as an arbitrarily complex collection of
   sources spread across the entire codebase
   - All the work of addressing the problems still remains to be done

 Contrast this with any basic ratchet-style approach (for example, capture
the list of mypy errors, then allow any PR that either removes some or
doesn't add any new ones):

   - At the outset, no application code breaks because nothing has actually
   changed
   - As work towards *fixing* the TODOs progresses, if it *does* break
   application code then at least applications have a chance at some benefit
   - As the work towards fixing the TODOs progresses, maintainers can
   easily look up what issues remain by consulting the list of errors that
   feeds the ratchet-style job.

The exact mechanism for the ratchet isn't the point here.  Maybe Mypy plays
more nicely with inline comments telling it to ignore something, I don't
know.  The point is:

   - A centralized list of stuff left to do is better than a mess smeared
   across the whole codebase
   - If we're going to risk breaking applications we should at least try to
   offer some value to them
   - We shouldn't make applications deal with every change twice when we
   could just disturb them once

Jean-Paul


> Specifically what I mean by "this genre of fix" is that you can always
> make mypy happy by transforming code like this:
>
> class Thing:
>     pass
>
> Thing().stuff()
>
>
> into code like this:
>
> class Thing:
>     @property
>     def stuff(self):
>         raise AttributeError("no such thing as `stuff`")
>
> Thing().stuff()
>
>
> The runtime behavior is the same, but now Mypy can't tell you about this
> class of bug any more.
>
> So, at some point, we should slowly unwind all `NotImplementedError`
> methods and find ways to either implement them for real or make their
> required use raise at import time.
>
> Finally: let's not develop an aversion to new tooling and change because
> it might create problems; experience over the last few years has shown me
> that Mypy can catch *tons* of real bugs and it's well worth getting the
> codebase to type check.  If we want to prevent breakages like this in the
> future, the answer is not to stop trying to get linters and typecheckers to
> run cleanly with arbitrary changes, but to figure out some kind of *continuous
> integration *solution that's actually continuous with our downstream
> dependencies
>
> If dependencies could start testing against Twisted trunk in some
> capacity, we could get notified close to when unintentionally breaking
> changes occur, and dependencies can let us know well before the release
> happens, and we can either revert or they can fix things if the error is on
> their end.  If initially, say, crossbar and matrix would like to work with
> us to set up some kind of repeatable pattern we can suggest to others, that
> would be great.
>
> -g
> _______________________________________________
> Twisted-Python mailing list
> Twisted-Python at twistedmatrix.com
> https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20210303/54796472/attachment-0001.htm>