This is an archive of the discontinued Mercurial Phabricator instance.

Differential D392

util: add base class for transactional context managers
ClosedPublic

Authored by martinvonz on Aug 14 2017, 7:46 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHGbbbbd3c30bfc: util: add base class for transactional context managers
Summary

We have at least three types with a close() and a release() method
where the close() method is supposed to be called on success and the
release() method is supposed to be called last, whether successful or
not. Two of them (transaction and dirstateguard) already have
identical implementations of enter and exit. Let's extract a
base class for this, so we reuse the code and so the third type
(transactionmanager) can also be used as a context manager.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Aug 14 2017, 7:46 PM
indygreg requested changes to this revision.Aug 14 2017, 9:32 PM
indygreg added a subscriber: indygreg.
indygreg added inline comments.
mercurial/util.py
598–602

How do you feel about changing this to use abc.ABCMeta so the interface is enforced as type creation time? I'm doing this in D332 and people seem to be on board.

This revision now requires changes to proceed.Aug 14 2017, 9:32 PM
martinvonz edited edge metadata.Aug 15 2017, 1:58 AM
martinvonz updated this revision to Diff 914.
martinvonz added inline comments.Aug 15 2017, 1:59 AM
mercurial/util.py
598–602

Sounds good. I had heard you mention abc before, but I was simply too lazy to bother checking how to do it. Now that I have an example: done! Let me know how it looks.

We should probably do that for matchers too, in case someone feels like it...

indygreg accepted this revision.Aug 15 2017, 2:09 AM
This revision is now accepted and ready to land.Aug 15 2017, 2:09 AM
Closed by commit rHGbbbbd3c30bfc: util: add base class for transactional context managers (authored by martinvonz). · Explain WhyAug 15 2017, 2:14 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/dirstateguard.py (13 lines)
Mmercurial/exchange.py (2 lines)
Mmercurial/transaction.py (12 lines)
Mmercurial/util.py (26 lines)

Diff 918

mercurial/dirstateguard.py

# dirstateguard.py - class to allow restoring dirstate after failure # dirstateguard.py - class to allow restoring dirstate after failure
# #
# Copyright 2005-2007 Matt Mackall <mpm@selenic.com> # Copyright 2005-2007 Matt Mackall <mpm@selenic.com>
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
from .i18n import _ from .i18n import _
from . import ( from . import (
error, error,
util,
) )
class dirstateguard(object): class dirstateguard(util.transactional):
'''Restore dirstate at unexpected failure. '''Restore dirstate at unexpected failure.
At the construction, this class does: At the construction, this class does:
- write current ``repo.dirstate`` out, and - write current ``repo.dirstate`` out, and
- save ``.hg/dirstate`` into the backup file - save ``.hg/dirstate`` into the backup file
This restores ``.hg/dirstate`` from backup file, if ``release()`` This restores ``.hg/dirstate`` from backup file, if ``release()``
def __del__(self): def __del__(self):
if self._active: # still active if self._active: # still active
# this may occur, even if this class is used correctly: # this may occur, even if this class is used correctly:
# for example, releasing other resources like transaction # for example, releasing other resources like transaction
# may raise exception before ``dirstateguard.release`` in # may raise exception before ``dirstateguard.release`` in
# ``release(tr, ....)``. # ``release(tr, ....)``.
self._abort() self._abort()
def __enter__(self):
return self
def __exit__(self, exc_type, exc_val, exc_tb):
try:
if exc_type is None:
self.close()
finally:
self.release()
def close(self): def close(self):
if not self._active: # already inactivated if not self._active: # already inactivated
msg = (_("can't close already inactivated backup: %s") msg = (_("can't close already inactivated backup: %s")
% self._backupname) % self._backupname)
raise error.Abort(msg) raise error.Abort(msg)
self._repo.dirstate.clearbackup(self._repo.currenttransaction(), self._repo.dirstate.clearbackup(self._repo.currenttransaction(),
self._backupname) self._backupname)

mercurial/exchange.py

@util.propertycache @util.propertycache
def remotebundle2caps(self): def remotebundle2caps(self):
return bundle2.bundle2caps(self.remote) return bundle2.bundle2caps(self.remote)
def gettransaction(self): def gettransaction(self):
# deprecated; talk to trmanager directly # deprecated; talk to trmanager directly
return self.trmanager.transaction() return self.trmanager.transaction()
class transactionmanager(object): class transactionmanager(util.transactional):
"""An object to manage the life cycle of a transaction """An object to manage the life cycle of a transaction
It creates the transaction on demand and calls the appropriate hooks when It creates the transaction on demand and calls the appropriate hooks when
closing the transaction.""" closing the transaction."""
def __init__(self, repo, source, url): def __init__(self, repo, source, url):
self.repo = repo self.repo = repo
self.source = source self.source = source
self.url = url self.url = url

mercurial/transaction.py

try: try:
for f in backupfiles: for f in backupfiles:
if opener.exists(f): if opener.exists(f):
opener.unlink(f) opener.unlink(f)
except (IOError, OSError, error.Abort) as inst: except (IOError, OSError, error.Abort) as inst:
# only pure backup file remains, it is sage to ignore any error # only pure backup file remains, it is sage to ignore any error
pass pass
class transaction(object): class transaction(util.transactional):
def __init__(self, report, opener, vfsmap, journalname, undoname=None, def __init__(self, report, opener, vfsmap, journalname, undoname=None,
after=None, createmode=None, validator=None, releasefn=None, after=None, createmode=None, validator=None, releasefn=None,
checkambigfiles=None): checkambigfiles=None):
"""Begin a new transaction """Begin a new transaction
Begins a new transaction that allows rolling back writes in the event of Begins a new transaction that allows rolling back writes in the event of
an exception. an exception.
def release(self): def release(self):
if self.count > 0: if self.count > 0:
self.usages -= 1 self.usages -= 1
# if the transaction scopes are left without being closed, fail # if the transaction scopes are left without being closed, fail
if self.count > 0 and self.usages == 0: if self.count > 0 and self.usages == 0:
self._abort() self._abort()
def __enter__(self):
return self
def __exit__(self, exc_type, exc_val, exc_tb):
try:
if exc_type is None:
self.close()
finally:
self.release()
def running(self): def running(self):
return self.count > 0 return self.count > 0
def addpending(self, category, callback): def addpending(self, category, callback):
"""add a callback to be called when the transaction is pending """add a callback to be called when the transaction is pending
The transaction will be given as callback's first argument. The transaction will be given as callback's first argument.

mercurial/util.py

"""Mercurial utility functions and platform specific implementations. """Mercurial utility functions and platform specific implementations.
This contains helper routines that are independent of the SCM core and This contains helper routines that are independent of the SCM core and
hide platform-specific details from the core. hide platform-specific details from the core.
""" """
from __future__ import absolute_import from __future__ import absolute_import
import abc
import bz2 import bz2
import calendar import calendar
import codecs import codecs
import collections import collections
import contextlib import contextlib
import datetime import datetime
import errno import errno
import gc import gc
if pycompat.ispypy: if pycompat.ispypy:
# __setitem__() isn't called as of PyPy 5.8.0 # __setitem__() isn't called as of PyPy 5.8.0
def update(self, src): def update(self, src):
if isinstance(src, dict): if isinstance(src, dict):
src = src.iteritems() src = src.iteritems()
for k, v in src: for k, v in src:
self[k] = v self[k] = v
class transactional(object):
"""Base class for making a transactional type into a context manager."""
__metaclass__ = abc.ABCMeta
@abc.abstractmethod
def close(self):
"""Successfully closes the transaction."""
indygregUnsubmitted
Not Done

How do you feel about changing this to use abc.ABCMeta so the interface is enforced as type creation time? I'm doing this in D332 and people seem to be on board.

indygreg: How do you feel about changing this to use `abc.ABCMeta` so the interface is enforced as type…
martinvonzAuthorUnsubmitted
Not Done

Sounds good. I had heard you mention abc before, but I was simply too lazy to bother checking how to do it. Now that I have an example: done! Let me know how it looks.

We should probably do that for matchers too, in case someone feels like it...

martinvonz: Sounds good. I had heard you mention abc before, but I was simply too lazy to bother checking…
@abc.abstractmethod
def release(self):
"""Marks the end of the transaction.
If the transaction has not been closed, it will be aborted.
"""
def __enter__(self):
return self
def __exit__(self, exc_type, exc_val, exc_tb):
try:
if exc_type is None:
self.close()
finally:
self.release()
@contextlib.contextmanager @contextlib.contextmanager
def acceptintervention(tr=None): def acceptintervention(tr=None):
"""A context manager that closes the transaction on InterventionRequired """A context manager that closes the transaction on InterventionRequired
If no transaction was provided, this simply runs the body and returns If no transaction was provided, this simply runs the body and returns
""" """
if not tr: if not tr:
yield yield