The same try/finally shape appears at more than one call site; a code
review comment says "don't forget to close/unlock/restore this"; a bug
report shows cleanup skipped on the exception path.
- Name the pair. What exactly is acquired, and what must run on exit? If you cannot state the release in one sentence, the block is doing too much to manage.
- Pick the form (see fundamentals): branching exit
logic → protocol class; one unconditional cleanup → generator form;
a runtime-sized set of cleanups →
ExitStack. - Write the exception path first. The manager exists for the failure case: decide what a mid-block exception means (discard? restore? both?) and test that before the happy path.
- Keep
__enter__cheap and__exit__unconditional. Acquisition failures should raise before the body runs; release must not depend on how far the body got. - Replace the call sites with
with, deleting their hand-rolledtry/finally. The diff should only remove lines.
- Generator form: the
yieldinsidetry/finally, always — the unit's first caveat exists because the failure is silent otherwise. ExitStack.callback(undo, ...)per step, thenpop_all()on success: transactional multi-step work where the commit is "don't run the undos" (shown in atomic_deploy).- A context manager that is also a decorator: subclass
contextlib.ContextDecorator, or stack@contextmanagerfunctions. contextlib.suppress(SomeError)instead oftry/except: pass— the intent gets a name.
yieldoutsidetry/finallyin a generator manager: cleanup runs only on the success path. The most common real-world defect in this pattern.- Returning
Truefrom__exit__(or swallowing infinally): exceptions vanish. Onlycontextlib.suppress-style managers should ever do it, and loudly. - Doing work in
__init__. Acquire in__enter__, or the manager cannot be reused and fails before thewithcan protect it. - One giant manager for several unrelated resources — compose small
ones with
ExitStackinstead; each stays testable alone.