Skip to content

Commit e0addef

Browse files
dhermesdhermes
authored
Merge pull request #2000 from dhermes/counter_set-happybase-just-put
Make counter_set in HappyBase agree with HBase impl. semantics
2 parents 8f3469c + 22dd5e8 commit e0addef

3 files changed

Lines changed: 52 additions & 25 deletions

File tree

‎gcloud/bigtable/happybase/__init__.py‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,26 +21,25 @@
2121
-------------------------
2222
2323
Some concepts from HBase/Thrift do not map directly to the Cloud
24-
Bigtable API. As a result, the following instance methods and functions
25-
could not be implemented:
24+
Bigtable API. As a result
2625
26+
* :meth:`Table.regions() <gcloud.bigtable.happybase.table.Table.regions>`
27+
could not be implemented since tables in Cloud Bigtable do not expose
28+
internal storage details
2729
* :meth:`Connection.enable_table() \
28-
<gcloud.bigtable.happybase.connection.Connection.enable_table>` - no
29-
concept of enabled/disabled
30+
<gcloud.bigtable.happybase.connection.Connection.enable_table>`
31+
does nothing since Cloud Bigtable has no concept of enabled/disabled
3032
* :meth:`Connection.disable_table() \
31-
<gcloud.bigtable.happybase.connection.Connection.disable_table>` - no
32-
concept of enabled/disabled
33+
<gcloud.bigtable.happybase.connection.Connection.disable_table>`
34+
does nothing since Cloud Bigtable has no concept of enabled/disabled
3335
* :meth:`Connection.is_table_enabled() \
3436
<gcloud.bigtable.happybase.connection.Connection.is_table_enabled>`
35-
- no concept of enabled/disabled
37+
always returns :data:`True` since Cloud Bigtable has no concept of
38+
enabled/disabled
3639
* :meth:`Connection.compact_table() \
37-
<gcloud.bigtable.happybase.connection.Connection.compact_table>` -
38-
table storage is opaque to user
39-
* :meth:`Table.regions() <gcloud.bigtable.happybase.table.Table.regions>`
40-
- tables in Cloud Bigtable do not expose internal storage details
41-
* :meth:`Table.counter_set() \
42-
<gcloud.bigtable.happybase.table.Table.counter_set>` - method can't
43-
be atomic, so we disable it
40+
<gcloud.bigtable.happybase.connection.Connection.compact_table>`
41+
does nothing since Cloud Bigtable handles table compactions automatically
42+
and does not expose an API for it
4443
* The ``__version__`` value for the HappyBase package is :data:`None`.
4544
However, it's worth nothing this implementation was based off HappyBase
4645
0.9.

‎gcloud/bigtable/happybase/table.py‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242

4343

4444
_WARN = warnings.warn
45+
_PACK_I64 = struct.Struct('>q').pack
4546
_UNPACK_I64 = struct.Struct('>q').unpack
4647
_SIMPLE_GC_RULES = (MaxAgeGCRule, MaxVersionsGCRule)
4748

@@ -531,9 +532,11 @@ def counter_get(self, row, column):
531532
def counter_set(self, row, column, value=0):
532533
"""Set a counter column to a specific value.
533534
534-
This method is provided in HappyBase, but we do not provide it here
535-
because it defeats the purpose of using atomic increment and decrement
536-
of a counter.
535+
.. note::
536+
537+
Be careful using this method. It can be useful for setting the
538+
initial value of a counter, but it defeats the purpose of using
539+
atomic increment and decrement.
537540
538541
:type row: str
539542
:param row: Row key for the row we are setting a counter in.
@@ -544,13 +547,8 @@ def counter_set(self, row, column, value=0):
544547
545548
:type value: int
546549
:param value: Value to set the counter to.
547-
548-
:raises: :class:`NotImplementedError <exceptions.NotImplementedError>`
549-
always
550550
"""
551-
raise NotImplementedError('Table.counter_set will not be implemented. '
552-
'Instead use the increment/decrement '
553-
'methods along with counter_get.')
551+
self.put(row, {column: _PACK_I64(value)})
554552

555553
def counter_inc(self, row, column, value=1):
556554
"""Atomically increment a counter column.

‎gcloud/bigtable/happybase/test_table.py‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -889,15 +889,45 @@ def _counter_inc_helper(self, row, column, value, commit_result):
889889
{tuple(column.split(':')): incremented_value})
890890

891891
def test_counter_set(self):
892+
import struct
893+
from gcloud._testing import _Monkey
894+
from gcloud.bigtable.happybase import table as MUT
895+
892896
name = 'table-name'
893897
connection = None
894898
table = self._makeOne(name, connection)
899+
batches_created = []
900+
901+
def make_batch(*args, **kwargs):
902+
result = _MockBatch(*args, **kwargs)
903+
batches_created.append(result)
904+
return result
895905

896906
row = 'row-key'
897907
column = 'fam:col1'
898908
value = 42
899-
with self.assertRaises(NotImplementedError):
900-
table.counter_set(row, column, value=value)
909+
with _Monkey(MUT, Batch=make_batch):
910+
result = table.counter_set(row, column, value=value)
911+
912+
# There is no return value.
913+
self.assertEqual(result, None)
914+
915+
# Check how the batch was created and used.
916+
batch, = batches_created
917+
self.assertTrue(isinstance(batch, _MockBatch))
918+
self.assertEqual(batch.args, (table,))
919+
expected_kwargs = {
920+
'timestamp': None,
921+
'batch_size': None,
922+
'transaction': False,
923+
'wal': MUT._WAL_SENTINEL,
924+
}
925+
self.assertEqual(batch.kwargs, expected_kwargs)
926+
# Make sure it was a successful context manager
927+
self.assertEqual(batch.exit_vals, [(None, None, None)])
928+
data = {column: struct.Struct('>q').pack(value)}
929+
self.assertEqual(batch.put_args, [(row, data)])
930+
self.assertEqual(batch.delete_args, [])
901931

902932
def test_counter_inc(self):
903933
import struct

0 commit comments

Comments
 (0)