Code review comment for lp:~rvb/maas/bug-1384001-redux

Revision history for this message
Julian Edwards (julian-edwards) wrote :

I appreciate what you're doing but I don't like the direction the code is taking here.

Firstly, IMO mass catch-all middleware is dangerous as developers will be unknowingly shielded from potentially immense cock-ups. Developers need to be aware of all the errors/exceptions coming out of a function all, and deal with them *as soon as possible*. This is a basic tenet of software development.

Secondly, I believe the original middleware fix did not work because the bug that I fixed in the change you're reverting was to stop a *500* error, not a 409 (conflict), which is what should have happened if that middleware was catching the PowerActionAlreadyInProgress correctly.

Thirdly, I don't think a global retry-after value is appropriate, it should be set by each site that raises the error, depending on the situation.

I'm happy to work with you on this if you want to chat about it.

review: Disapprove

« Back to merge proposal