From a0569f166938dcdc4af496db237aaa3038ba62a5 Mon Sep 17 00:00:00 2001 From: Kai Lautaportti Date: Wed, 3 Jun 2015 09:26:18 -0700 Subject: [PATCH 1/3] Expose conflicting data in case of LWTException Adds a custom attribute, `existing`, to the LWTException instance which allows the application layer to inspect the values read from Cassandra which conflicted with the attempted conditional write. The documentation already referred to the `existing` attribute but it was not implemented. --- cassandra/cqlengine/query.py | 14 +++++++-- docs/api/cassandra/cqlengine/models.rst | 8 ++--- docs/api/cassandra/cqlengine/query.rst | 1 + .../integration/cqlengine/test_ifnotexists.py | 19 ++++++++++-- .../integration/cqlengine/test_transaction.py | 29 ++++++++++++++++--- 5 files changed, 59 insertions(+), 12 deletions(-) diff --git a/cassandra/cqlengine/query.py b/cassandra/cqlengine/query.py index 1f4a1c3b..dc64a005 100644 --- a/cassandra/cqlengine/query.py +++ b/cassandra/cqlengine/query.py @@ -40,7 +40,17 @@ class IfNotExistsWithCounterColumn(CQLEngineException): class LWTException(CQLEngineException): - pass + """Lightweight transaction exception. + + This exception will be raised when a write using an `IF` clause could not be + applied due to existing data violating the condition. The existing data is + available through the ``existing`` attribute. + + :param existing: The current state of the data which prevented the write. + """ + def __init__(self, existing): + super(LWTException, self).__init__(self) + self.existing = existing class DoesNotExist(QueryException): @@ -58,7 +68,7 @@ def check_applied(result): applied to database. """ if result and '[applied]' in result[0] and not result[0]['[applied]']: - raise LWTException('') + raise LWTException(result[0]) class AbstractQueryableColumn(UnicodeMixin): diff --git a/docs/api/cassandra/cqlengine/models.rst b/docs/api/cassandra/cqlengine/models.rst index b8d8a71a..aad91461 100644 --- a/docs/api/cassandra/cqlengine/models.rst +++ b/docs/api/cassandra/cqlengine/models.rst @@ -48,7 +48,6 @@ Model See the `list of supported table properties for more information `_. - .. attribute:: __options__ For example: @@ -89,7 +88,7 @@ Model object is determined by its primary key(s). And please note using this flag would incur performance cost. - if the insertion didn't applied, a LWTException exception would be raised. + If the insertion isn't applied, a :class:`~cassandra.cqlengine.query.LWTException` is raised. .. code-block:: python @@ -111,7 +110,7 @@ Model Simply specify the column(s) and the expected value(s). As with if_not_exists, this incurs a performance cost. - If the insertion isn't applied, a LWTException is raised + If the insertion isn't applied, a :class:`~cassandra.cqlengine.query.LWTException` is raised. .. code-block:: python @@ -119,7 +118,8 @@ Model try: t.iff(count=5).update('other text') except LWTException as e: - # handle failure + # handle failure case + print e.existing # existing object .. automethod:: get diff --git a/docs/api/cassandra/cqlengine/query.rst b/docs/api/cassandra/cqlengine/query.rst index df823704..e67df5df 100644 --- a/docs/api/cassandra/cqlengine/query.rst +++ b/docs/api/cassandra/cqlengine/query.rst @@ -40,3 +40,4 @@ The methods here are used to filter, order, and constrain results. .. autoclass:: MultipleObjectsReturned +.. autoclass:: LWTException diff --git a/tests/integration/cqlengine/test_ifnotexists.py b/tests/integration/cqlengine/test_ifnotexists.py index 3fc95d2d..42df821d 100644 --- a/tests/integration/cqlengine/test_ifnotexists.py +++ b/tests/integration/cqlengine/test_ifnotexists.py @@ -81,9 +81,17 @@ class IfNotExistsInsertTests(BaseIfNotExistsTest): id = uuid4() TestIfNotExistsModel.create(id=id, count=8, text='123456789') - with self.assertRaises(LWTException): + + with self.assertRaises(LWTException) as assertion: TestIfNotExistsModel.if_not_exists().create(id=id, count=9, text='111111111111') + self.assertEquals(assertion.exception.existing, { + 'count': 8, + 'id': id, + 'text': '123456789', + '[applied]': False, + }) + q = TestIfNotExistsModel.objects(id=id) self.assertEqual(len(q), 1) @@ -117,9 +125,16 @@ class IfNotExistsInsertTests(BaseIfNotExistsTest): b = BatchQuery() TestIfNotExistsModel.batch(b).if_not_exists().create(id=id, count=9, text='111111111111') - with self.assertRaises(LWTException): + with self.assertRaises(LWTException) as assertion: b.execute() + self.assertEquals(assertion.exception.existing, { + 'count': 8, + 'id': id, + 'text': '123456789', + '[applied]': False, + }) + q = TestIfNotExistsModel.objects(id=id) self.assertEqual(len(q), 1) diff --git a/tests/integration/cqlengine/test_transaction.py b/tests/integration/cqlengine/test_transaction.py index b3d7ea0f..3f7a19ee 100644 --- a/tests/integration/cqlengine/test_transaction.py +++ b/tests/integration/cqlengine/test_transaction.py @@ -29,8 +29,9 @@ from cassandra.cqlengine.statements import TransactionClause from tests.integration.cqlengine.base import BaseCassEngTestCase from tests.integration import CASSANDRA_VERSION + class TestTransactionModel(Model): - id = columns.UUID(primary_key=True, default=lambda:uuid4()) + id = columns.UUID(primary_key=True, default=uuid4) count = columns.Integer() text = columns.Text(required=False) @@ -71,7 +72,14 @@ class TestTransaction(BaseCassEngTestCase): t = TestTransactionModel.create(text='blah blah') t.text = 'new blah' t = t.iff(text='something wrong') - self.assertRaises(LWTException, t.save) + + with self.assertRaises(LWTException) as assertion: + t.save() + + self.assertEqual(assertion.exception.existing, { + 'text': 'blah blah', + '[applied]': False, + }) def test_blind_update(self): t = TestTransactionModel.create(text='blah blah') @@ -89,7 +97,13 @@ class TestTransaction(BaseCassEngTestCase): t.text = 'something else' uid = t.id qs = TestTransactionModel.objects(id=uid).iff(text='Not dis!') - self.assertRaises(LWTException, qs.update, text='this will never work') + with self.assertRaises(LWTException) as assertion: + qs.update(text='this will never work') + + self.assertEqual(assertion.exception.existing, { + 'text': 'blah blah', + '[applied]': False, + }) def test_transaction_clause(self): tc = TransactionClause('some_value', 23) @@ -109,7 +123,14 @@ class TestTransaction(BaseCassEngTestCase): b = BatchQuery() updated.batch(b).iff(count=6).update(text='and another thing') - self.assertRaises(LWTException, b.execute) + with self.assertRaises(LWTException) as assertion: + b.execute() + + self.assertEqual(assertion.exception.existing, { + 'id': id, + 'count': 5, + '[applied]': False, + }) updated = TestTransactionModel.objects(id=id).first() self.assertEqual(updated.text, 'something else') From 94fe1fce992fc0541c60578f4df78e571077a7a7 Mon Sep 17 00:00:00 2001 From: Adam Holmberg Date: Tue, 23 Feb 2016 13:30:35 -0600 Subject: [PATCH 2/3] ResultSet.was_applied property for LWT results. --- cassandra/cluster.py | 25 +++++++++++++++++++++++-- cassandra/cqlengine/query.py | 10 ++++++---- tests/unit/test_resultset.py | 27 +++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/cassandra/cluster.py b/cassandra/cluster.py index 1954a139..13afecd2 100644 --- a/cassandra/cluster.py +++ b/cassandra/cluster.py @@ -70,8 +70,8 @@ from cassandra.pool import (Host, _ReconnectionHandler, _HostReconnectionHandler HostConnectionPool, HostConnection, NoConnectionsAvailable) from cassandra.query import (SimpleStatement, PreparedStatement, BoundStatement, - BatchStatement, bind_params, QueryTrace, Statement, - named_tuple_factory, dict_factory, FETCH_SIZE_UNSET) + BatchStatement, bind_params, QueryTrace, + named_tuple_factory, dict_factory, tuple_factory, FETCH_SIZE_UNSET) def _is_eventlet_monkey_patched(): @@ -3409,3 +3409,24 @@ class ResultSet(object): See :meth:`.ResponseFuture.get_all_query_traces` for details. """ return self.response_future.get_all_query_traces(max_wait_sec_per) + + @property + def was_applied(self): + """ + For LWT results, returns whether the transaction was applied. + + Result is indeterminate if called on a result that was not an LWT request. + + Only valid when one of tne of the internal row factories is in use. + """ + if self.response_future.row_factory not in (named_tuple_factory, dict_factory, tuple_factory): + raise RuntimeError("Cannot determine LWT result with row factory %s" % (self.response_future.row_factsory,)) + if len(self.current_rows) != 1: + raise RuntimeError("LWT result should have exactly one row. This has %d." % (len(self.current_rows))) + + row = self.current_rows[0] + if isinstance(row, tuple): + return row[0] + else: + return row['[applied]'] + diff --git a/cassandra/cqlengine/query.py b/cassandra/cqlengine/query.py index dc64a005..d356bf5e 100644 --- a/cassandra/cqlengine/query.py +++ b/cassandra/cqlengine/query.py @@ -63,11 +63,13 @@ class MultipleObjectsReturned(QueryException): def check_applied(result): """ - check if result contains some column '[applied]' with false value, - if that value is false, it means our light-weight transaction didn't - applied to database. + Raises LWTException if it looks like a failed LWT request. """ - if result and '[applied]' in result[0] and not result[0]['[applied]']: + try: + applied = result.was_applied + except Exception: + applied = True # result was not LWT form + if not applied: raise LWTException(result[0]) diff --git a/tests/unit/test_resultset.py b/tests/unit/test_resultset.py index 252b6e05..2deeb30f 100644 --- a/tests/unit/test_resultset.py +++ b/tests/unit/test_resultset.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from cassandra.query import named_tuple_factory, dict_factory, tuple_factory try: import unittest2 as unittest @@ -161,3 +162,29 @@ class ResultSetTests(unittest.TestCase): def test_bool(self): self.assertFalse(ResultSet(Mock(has_more_pages=False), [])) self.assertTrue(ResultSet(Mock(has_more_pages=False), [1])) + + def test_was_applied(self): + # unknown row factory raises + with self.assertRaises(RuntimeError): + ResultSet(Mock(), []).was_applied + + response_future = Mock(row_factory=named_tuple_factory) + + # no row + with self.assertRaises(RuntimeError): + ResultSet(response_future, []).was_applied + + # too many rows + with self.assertRaises(RuntimeError): + ResultSet(response_future, [tuple(), tuple()]).was_applied + + # various internal row factories + for row_factory in (named_tuple_factory, tuple_factory): + for applied in (True, False): + rs = ResultSet(Mock(row_factory=row_factory), [(applied,)]) + self.assertEqual(rs.was_applied, applied) + + row_factory = dict_factory + for applied in (True, False): + rs = ResultSet(Mock(row_factory=row_factory), [{'[applied]': applied}]) + self.assertEqual(rs.was_applied, applied) From 5e102605cdff5afd987c463e27b442b94a4f755f Mon Sep 17 00:00:00 2001 From: Adam Holmberg Date: Tue, 23 Feb 2016 13:32:37 -0600 Subject: [PATCH 3/3] PYTHON-336 test tweaks and changelog --- CHANGELOG.rst | 1 + docs/api/cassandra/cqlengine/models.rst | 2 +- tests/integration/cqlengine/test_ifnotexists.py | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 54355a40..53c84afe 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,6 +6,7 @@ Features * Pass name of server auth class to AuthProvider (PYTHON-454) * Surface schema agreed flag for DDL statements (PYTHON-458) * Automatically convert float and int to Decimal on serialization (PYTHON-468) +* Expose prior state information via cqlengine LWTException (github #343) Bug Fixes --------- diff --git a/docs/api/cassandra/cqlengine/models.rst b/docs/api/cassandra/cqlengine/models.rst index aad91461..ff198a49 100644 --- a/docs/api/cassandra/cqlengine/models.rst +++ b/docs/api/cassandra/cqlengine/models.rst @@ -96,7 +96,7 @@ Model TestIfNotExistsModel.if_not_exists().create(id=id, count=9, text='111111111111') except LWTException as e: # handle failure case - print e.existing # existing object + print e.existing # dict containing LWT result fields This method is supported on Cassandra 2.0 or later. diff --git a/tests/integration/cqlengine/test_ifnotexists.py b/tests/integration/cqlengine/test_ifnotexists.py index 42df821d..43d53319 100644 --- a/tests/integration/cqlengine/test_ifnotexists.py +++ b/tests/integration/cqlengine/test_ifnotexists.py @@ -85,7 +85,7 @@ class IfNotExistsInsertTests(BaseIfNotExistsTest): with self.assertRaises(LWTException) as assertion: TestIfNotExistsModel.if_not_exists().create(id=id, count=9, text='111111111111') - self.assertEquals(assertion.exception.existing, { + self.assertEqual(assertion.exception.existing, { 'count': 8, 'id': id, 'text': '123456789', @@ -128,7 +128,7 @@ class IfNotExistsInsertTests(BaseIfNotExistsTest): with self.assertRaises(LWTException) as assertion: b.execute() - self.assertEquals(assertion.exception.existing, { + self.assertEqual(assertion.exception.existing, { 'count': 8, 'id': id, 'text': '123456789',