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

Glyph glyph at twistedmatrix.com
Wed Mar 3 11:59:12 MST 2021


> On Mar 3, 2021, at 4:58 AM, Jean-Paul Calderone <exarkun at twistedmatrix.com> wrote:
> 
> 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):

In principle, I agree.  And some coarse-grained ratchets are absolutely worth doing, when they're supported by the tools - for example, we have one of these with the way we're handling `disallow_untyped_defs` right now.  I wish tools like Mypy would build in ratcheting infrastructure that was easy to use and convenient to deploy.

But in practice this type of ratchet involves a "# type: ignore" at every single call site, as well as delaying shipping stubs or maintaining separate stubs.  As well as building the maladaptive habit that #type:ignore or cast()ing your way out of type errors is the right way to address them in individual features and getting every code reviewer used to that.
> 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:

The "exact mechanism for the ratchet" introduces enough hard problems that it inevitably delays tooling adoption for months or years, reduces the benefits of the tooling adoption (what Twisted users really want are comprehensive stubs, less than Twisted to type-check internally, I think, and this lets us ship them sooner in at least a mostly-usable state).
> 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

Overall I very much appreciate your criticism and I wish that I could provide clear and concrete steps towards a functioning ratchet for all of these types of tools, but in practice the minor issues caused by the boil-the-ocean approach are an order of magnitude easier to address (and cheaper to leave un-addressed) than the effort required to build ratchets for our contributors that aren't a total nightmare to maintain and interact with.

Case in point: running `black` was a giant earthquake in the codebase but the pain associated with that was a drop in the bucket compared to the lingering, years-long nightmare of our attempt to build a style ratchet with twistedchecker.  In principle what we were trying to do with twistedchecker was very simple, but the practicalities ate us alive.

You could of course quite rightly argue that there were multiple conflated issues at play there!  But that's also the point: there's always a lot of complexity that comes along with trying to do codemod migrations "the right way" when that's going against the grain of the rest of the ecosystem.

I don't love that we've foisted this off onto our users, but I suspect Tobias spent a lot less time fixing the bug here than we've spent writing emails about it at this point ;-).

I look forward to the day where I'm wrong about all of this and every new code quality tool comes with a super easy-to-deploy ratchet, though.

-g

-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20210303/3c71ecab/attachment-0001.htm>


More information about the Twisted-Python mailing list