launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05021
[Merge] lp:~wgrant/launchpad/ztm-murder into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/ztm-murder into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/ztm-murder/+merge/75949
This branch is hopefully the penultimate phase of my ZopelessTransactionManager extermination campaign. It has been reduced to two public methods: initZopeless and uninstall. It is no longer a transaction manager -- just a database configuration modifier.
LaunchpadScript and ZopelessLayer set self.txn = transaction instead of using ZTM, and ZTM's begin/commit/abort methods are gone. sqlbase's begin/rollback/expire_from_cache convenience methods have also died, but commit() remains because it has many callsites, to be purged in another branch.
initZopeless also no longer returns ZTM.
--
https://code.launchpad.net/~wgrant/launchpad/ztm-murder/+merge/75949
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/ztm-murder into lp:launchpad.
=== modified file 'lib/canonical/database/ftests/script_isolation.py'
--- lib/canonical/database/ftests/script_isolation.py 2011-09-19 04:08:10 +0000
+++ lib/canonical/database/ftests/script_isolation.py 2011-09-19 04:53:24 +0000
@@ -16,6 +16,8 @@
'ignore', '.*(md5|sha|sets)', DeprecationWarning,
)
+import transaction
+
from canonical.database.sqlbase import (
cursor,
ISOLATION_LEVEL_SERIALIZABLE,
@@ -31,8 +33,8 @@
cur.execute("SHOW transaction_isolation")
print cur.fetchone()[0]
- txn.abort()
- txn.begin()
+ transaction.abort()
+ transaction.begin()
cur = cursor()
cur.execute("UPDATE Person SET homepage_content='bar' WHERE name='mark'")
@@ -40,12 +42,14 @@
print cur.fetchone()[0]
# First confirm the default isolation level
-txn = ZopelessTransactionManager.initZopeless(dbuser='launchpad_main')
+ZopelessTransactionManager.initZopeless(dbuser='launchpad_main')
check()
-txn.uninstall()
+ZopelessTransactionManager.uninstall()
-txn = ZopelessTransactionManager.initZopeless(
+# We run the checks twice to ensure that both methods of setting the
+# isolation level stick across transaction boundaries.
+ZopelessTransactionManager.initZopeless(
dbuser='launchpad_main',
isolation=ISOLATION_LEVEL_SERIALIZABLE)
check()
-txn.uninstall()
+ZopelessTransactionManager.uninstall()
=== modified file 'lib/canonical/database/ftests/test_zopelesstransactionmanager.txt'
--- lib/canonical/database/ftests/test_zopelesstransactionmanager.txt 2011-09-19 04:08:10 +0000
+++ lib/canonical/database/ftests/test_zopelesstransactionmanager.txt 2011-09-19 04:53:24 +0000
@@ -12,19 +12,12 @@
class method.
>>> from canonical.database.sqlbase import ZopelessTransactionManager
- >>> ztm = ZopelessTransactionManager.initZopeless(
- ... dbuser='launchpad_main')
-
-The returned transaction manager happens to be the same as the
-ZopelessTransactionManager class:
-
- >>> ztm is ZopelessTransactionManager
- True
+ >>> ZopelessTransactionManager.initZopeless(dbuser='launchpad_main')
After initZopeless() has been called, the '_installed' attribute of
ZopelessTransactionManager will be set to the transaction manager:
- >>> ZopelessTransactionManager._installed is ztm
+ >>> ZopelessTransactionManager._installed is ZopelessTransactionManager
True
The initZopeless() call defaults to read committed isolation:
@@ -35,47 +28,30 @@
>>> print c.fetchone()[0]
read committed
-We can commit transactions:
-
- >>> c.execute("UPDATE person SET name = 'something' WHERE id = 1")
- >>> ztm.commit()
-
-We can also explicitly start transactions with begin(), and abort
-transactions:
-
- >>> ztm.begin()
- >>> c = cursor()
- >>> c.execute("UPDATE person SET name = 'blah' WHERE id = 1")
- >>> ztm.abort()
- >>> c = cursor()
- >>> c.execute("SELECT name FROM person WHERE id = 1")
- >>> print c.fetchone()[0]
- something
-
The uninstall() method can be used to uninstall the transaction
manager:
- >>> ztm.uninstall()
+ >>> ZopelessTransactionManager.uninstall()
>>> print ZopelessTransactionManager._installed
None
We can log in as alternative users with initZopeless():
- >>> ztm = ZopelessTransactionManager.initZopeless(dbuser='testadmin')
+ >>> ZopelessTransactionManager.initZopeless(dbuser='testadmin')
>>> c = cursor()
>>> c.execute("SELECT current_user")
>>> print c.fetchone()[0]
testadmin
- >>> ztm.uninstall()
+ >>> ZopelessTransactionManager.uninstall()
Or we can specify other transaction isolation modes:
>>> from canonical.database.sqlbase import (
... ISOLATION_LEVEL_SERIALIZABLE)
- >>> ztm = ZopelessTransactionManager.initZopeless(
+ >>> ZopelessTransactionManager.initZopeless(
... dbuser='librarian', isolation=ISOLATION_LEVEL_SERIALIZABLE)
>>> c = cursor()
>>> c.execute("SHOW transaction_isolation")
>>> print c.fetchone()[0]
serializable
- >>> ztm.uninstall()
+ >>> ZopelessTransactionManager.uninstall()
=== modified file 'lib/canonical/database/sqlbase.py'
--- lib/canonical/database/sqlbase.py 2011-09-18 08:11:37 +0000
+++ lib/canonical/database/sqlbase.py 2011-09-19 04:53:24 +0000
@@ -4,7 +4,6 @@
__metaclass__ = type
__all__ = [
'alreadyInstalledMsg',
- 'begin',
'block_implicit_flushes',
'clear_current_connection_cache',
'commit',
@@ -12,7 +11,6 @@
'connect',
'convert_storm_clause_to_string',
'cursor',
- 'expire_from_cache',
'flush_database_caches',
'flush_database_updates',
'get_transaction_timestamp',
@@ -25,7 +23,6 @@
'quoteIdentifier',
'quote_identifier',
'reset_store',
- 'rollback',
'session_store',
'SQLBase',
'sqlvalues',
@@ -328,7 +325,6 @@
cls._isolation = isolation
cls._reset_stores()
cls._installed = cls
- return cls._installed
@staticmethod
def _reset_stores():
@@ -370,21 +366,6 @@
cls._reset_stores()
cls._installed = None
- @staticmethod
- def begin():
- """Begin a transaction."""
- transaction.begin()
-
- @staticmethod
- def commit():
- """Commit the current transaction."""
- transaction.commit()
-
- @staticmethod
- def abort():
- """Abort the current transaction."""
- transaction.abort()
-
def clear_current_connection_cache():
"""Clear SQLObject's object cache. SQLObject compatibility - DEPRECATED.
@@ -392,12 +373,6 @@
_get_sqlobject_store().invalidate()
-def expire_from_cache(obj):
- """Expires a single object from the SQLObject cache.
- SQLObject compatibility - DEPRECATED."""
- _get_sqlobject_store().invalidate(obj)
-
-
def get_transaction_timestamp():
"""Get the timestamp for the current transaction on the MAIN DEFAULT
store. DEPRECATED - if needed it should become a method on the store.
@@ -695,18 +670,7 @@
return mergeFunctionMetadata(func, reset_store_decorator)
-# Some helpers intended for use with initZopeless. These allow you to avoid
-# passing the transaction manager all through your code.
-
-def begin():
- """Begins a transaction."""
- transaction.begin()
-
-
-def rollback():
- transaction.abort()
-
-
+# DEPRECATED -- use transaction.commit() directly.
def commit():
transaction.commit()
=== modified file 'lib/canonical/database/tests/test_zopeless_transaction_manager.py'
--- lib/canonical/database/tests/test_zopeless_transaction_manager.py 2011-09-13 12:14:00 +0000
+++ lib/canonical/database/tests/test_zopeless_transaction_manager.py 2011-09-19 04:53:24 +0000
@@ -50,17 +50,16 @@
warnings.warn = self.warn_hooked
self.warned = False
try:
- # Calling initZopeless with the same arguments twice should return
- # the exact same object twice, but also emit a warning.
+ # Calling initZopeless with the same arguments twice should emit
+ # a warning.
try:
- tm1 = ZopelessTransactionManager.initZopeless(
- dbuser='launchpad')
- tm2 = ZopelessTransactionManager.initZopeless(
- dbuser='launchpad')
- self.failUnless(tm1 is tm2)
+ ZopelessTransactionManager.initZopeless(
+ dbuser='launchpad')
+ ZopelessTransactionManager.initZopeless(
+ dbuser='launchpad')
self.failUnless(self.warned)
finally:
- tm1.uninstall()
+ ZopelessTransactionManager.uninstall()
finally:
# Put the warnings module back the way we found it.
warnings.warn = original_warn
=== modified file 'lib/canonical/testing/layers.py'
--- lib/canonical/testing/layers.py 2011-09-18 07:15:22 +0000
+++ lib/canonical/testing/layers.py 2011-09-19 04:53:24 +0000
@@ -1510,7 +1510,7 @@
"""
isSetUp = False
- txn = ZopelessTransactionManager
+ txn = transaction
@classmethod
@profiled
=== modified file 'lib/lp/archiveuploader/tests/nascentupload-epoch-handling.txt'
--- lib/lp/archiveuploader/tests/nascentupload-epoch-handling.txt 2010-10-14 17:37:39 +0000
+++ lib/lp/archiveuploader/tests/nascentupload-epoch-handling.txt 2011-09-19 04:53:24 +0000
@@ -78,8 +78,8 @@
Clean up this change and carry on:
- >>> from canonical.database.sqlbase import rollback
- >>> rollback()
+ >>> import transaction
+ >>> transaction.abort()
=== Collision in queue DONE ===
@@ -182,7 +182,7 @@
Clean up this change and carry on:
- >>> rollback()
+ >>> transaction.abort()
== Dealing with Epochs and diverging binary versions ==
=== modified file 'lib/lp/bugs/doc/bug-export.txt'
--- lib/lp/bugs/doc/bug-export.txt 2010-10-18 22:24:59 +0000
+++ lib/lp/bugs/doc/bug-export.txt 2011-09-19 04:53:24 +0000
@@ -81,10 +81,8 @@
the bugs for a product. The export_bugtasks() function does this by
successively serialising each of the tasks for that product.
- >>> from canonical.database.sqlbase import ZopelessTransactionManager
- >>> ztm = ZopelessTransactionManager._installed
-
- >>> export_bugtasks(ztm, firefox, sys.stdout)
+ >>> import transaction
+ >>> export_bugtasks(transaction, firefox, sys.stdout)
<launchpad-bugs xmlns="https://launchpad.net/xmlns/2006/bugs";>
<bug id="1">
...
@@ -128,7 +126,7 @@
... 'Added attachment', 'hello.txt',
... description='"Hello World" attachment')
<BugAttachment ...>
- >>> ztm.commit()
+ >>> transaction.commit()
A reference to the attachment is included with the new comment with
the attachment contents encoded using base-64:
@@ -163,13 +161,13 @@
>>> bug1.setPrivate(True, getUtility(ILaunchBag).user)
True
- >>> ztm.commit()
+ >>> transaction.commit()
Now we'll do a dump not including private bugs:
>>> output = StringIO()
- >>> export_bugtasks(ztm, firefox, output)
+ >>> export_bugtasks(transaction, firefox, output)
>>> '<bug id="1">' in output.getvalue()
False
@@ -177,6 +175,6 @@
However, bug #1 will appear in the export if we include private bugs:
>>> output = StringIO()
- >>> export_bugtasks(ztm, firefox, output, include_private=True)
+ >>> export_bugtasks(transaction, firefox, output, include_private=True)
>>> '<bug id="1">' in output.getvalue()
True
=== modified file 'lib/lp/bugs/doc/checkwatches-cli-switches.txt'
--- lib/lp/bugs/doc/checkwatches-cli-switches.txt 2011-03-23 16:28:51 +0000
+++ lib/lp/bugs/doc/checkwatches-cli-switches.txt 2011-09-19 04:53:24 +0000
@@ -5,8 +5,7 @@
the updateBugTrackers() method.
>>> import transaction
- >>> from canonical.database.sqlbase import (
- ... cursor, sqlvalues, ZopelessTransactionManager)
+ >>> from canonical.database.sqlbase import cursor, sqlvalues
>>> from canonical.database.constants import UTC_NOW
>>> from lp.services.log.logger import FakeLogger
>>> from lp.bugs.scripts.checkwatches import CheckwatchesMaster
@@ -19,8 +18,7 @@
... sqlvalues(UTC_NOW))
>>> transaction.commit()
- >>> transactionmgr = ZopelessTransactionManager._installed
- >>> updater = CheckwatchesMaster(transactionmgr, logger=FakeLogger())
+ >>> updater = CheckwatchesMaster(transaction, logger=FakeLogger())
>>> updater.updateBugTrackers(['debbugs', 'gnome-bugzilla'])
DEBUG...No watches to update on http://bugs.debian.org
@@ -52,7 +50,7 @@
... def __init__(self, name, dbuser=None, test_args=None):
... super(TestCheckWatchesCronScript, self).__init__(
... name, dbuser, test_args)
- ... self.txn = ZopelessTransactionManager
+ ... self.txn = transaction
...
... def handle_options(self):
... self.logger = FakeLogger()
=== modified file 'lib/lp/bugs/mail/handler.py'
--- lib/lp/bugs/mail/handler.py 2011-09-15 17:25:54 +0000
+++ lib/lp/bugs/mail/handler.py 2011-09-19 04:53:24 +0000
@@ -13,11 +13,11 @@
from lazr.lifecycle.event import ObjectCreatedEvent
from lazr.lifecycle.interfaces import IObjectCreatedEvent
+import transaction
from zope.component import getUtility
from zope.event import notify
from zope.interface import implements
-from canonical.database.sqlbase import rollback
from canonical.launchpad.helpers import get_email_template
from canonical.launchpad.mailnotification import (
MailWrapper,
@@ -300,7 +300,7 @@
processing_errors.append((error, command))
if error.stop_processing:
commands = []
- rollback()
+ transaction.abort()
else:
continue
@@ -431,7 +431,7 @@
notify(bugtask_event)
def handleNoAffectsTarget(self):
- rollback()
+ transaction.abort()
raise IncomingEmailError(
get_error_message(
'no-affects-target-on-submit.txt',
=== modified file 'lib/lp/codehosting/scanner/tests/test_bzrsync.py'
--- lib/lp/codehosting/scanner/tests/test_bzrsync.py 2011-08-18 15:33:25 +0000
+++ lib/lp/codehosting/scanner/tests/test_bzrsync.py 2011-09-19 04:53:24 +0000
@@ -24,6 +24,7 @@
from zope.security.proxy import removeSecurityProxy
from canonical.config import config
+from canonical.database.sqlbase import ZopelessTransactionManager
from canonical.launchpad.interfaces.lpstorm import IStore
from canonical.testing.layers import LaunchpadZopelessLayer
from lp.code.interfaces.branchjob import IRosettaUploadJobSource
@@ -65,7 +66,7 @@
def _run_with_different_user(f):
def decorated(*args, **kwargs):
- current_user = LaunchpadZopelessLayer.txn._dbuser
+ current_user = ZopelessTransactionManager._dbuser
if current_user == username:
return f(*args, **kwargs)
LaunchpadZopelessLayer.switchDbUser(username)
=== modified file 'lib/lp/services/scripts/base.py'
--- lib/lp/services/scripts/base.py 2011-09-18 07:20:04 +0000
+++ lib/lp/services/scripts/base.py 2011-09-19 04:53:24 +0000
@@ -30,6 +30,7 @@
LockAlreadyAcquired,
)
import pytz
+import transaction
from zope.component import getUtility
from canonical.config import config, dbconfig
@@ -359,8 +360,9 @@
if dbuser is None:
connstr = ConnectionString(dbconfig.main_master)
dbuser = connstr.user or dbconfig.dbuser
- self.txn = ZopelessTransactionManager.initZopeless(
+ ZopelessTransactionManager.initZopeless(
dbuser=dbuser, isolation=isolation)
+ self.txn = transaction
def record_activity(self, date_started, date_completed):
"""Hook to record script activity."""
=== modified file 'lib/lp/testing/dbuser.py'
--- lib/lp/testing/dbuser.py 2011-02-07 20:41:06 +0000
+++ lib/lp/testing/dbuser.py 2011-09-19 04:53:24 +0000
@@ -11,29 +11,34 @@
from contextlib import contextmanager
+import transaction
+
+from canonical.database.sqlbase import ZopelessTransactionManager
from canonical.testing.layers import LaunchpadZopelessLayer
+
@contextmanager
def dbuser(temporary_name):
"""A context manager that temporarily changes the dbuser.
-
+
Use with the LaunchpadZopelessLayer layer and subclasses.
-
+
temporary_name is the name of the dbuser that should be in place for the
code in the "with" block.
"""
- restore_name = LaunchpadZopelessLayer.txn._dbuser
- LaunchpadZopelessLayer.txn.commit()
+ restore_name = ZopelessTransactionManager._dbuser
+ transaction.commit()
# Note that this will raise an assertion error if the
# LaunchpadZopelessLayer is not already set up.
LaunchpadZopelessLayer.switchDbUser(temporary_name)
yield
- LaunchpadZopelessLayer.txn.commit()
+ transaction.commit()
LaunchpadZopelessLayer.switchDbUser(restore_name)
+
def lp_dbuser():
"""A context manager that temporarily changes to the launchpad dbuser.
-
+
Use with the LaunchpadZopelessLayer layer and subclasses.
"""
return dbuser('launchpad')
=== modified file 'lib/lp/translations/utilities/tests/helpers.py'
--- lib/lp/translations/utilities/tests/helpers.py 2011-05-27 19:53:20 +0000
+++ lib/lp/translations/utilities/tests/helpers.py 2011-09-19 04:53:24 +0000
@@ -13,10 +13,6 @@
import transaction
from zope.component import getUtility
-from canonical.database.sqlbase import (
- commit,
- ZopelessTransactionManager,
- )
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.services.log.logger import FakeLogger
from lp.translations.enums import RosettaImportStatus
@@ -66,10 +62,7 @@
potemplate=potemplate)
target = potemplate
# Allow Librarian to see the change.
- if ZopelessTransactionManager._installed is None:
- transaction.commit()
- else:
- commit()
+ transaction.commit()
entry.setStatus(RosettaImportStatus.APPROVED,
getUtility(ILaunchpadCelebrities).rosetta_experts)
Follow ups
