Skip to content

Commit

Permalink
Making HappyBase compact_table() a no-op.
Browse files Browse the repository at this point in the history
Also fixing some lint related errors (not using arguments
and not using self).
  • Loading branch information
dhermes committed Jul 19, 2016
1 parent 6bd2442 commit 699c027
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 22 deletions.
40 changes: 24 additions & 16 deletions gcloud/bigtable/happybase/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,12 @@
_BASE_DISABLE = 'Cloud Bigtable has no concept of enabled / disabled tables.'
_DISABLE_DELETE_MSG = ('The disable argument should not be used in '
'delete_table(). ') + _BASE_DISABLE
_ENABLE_MSG = 'Connection.enable_table() was called, but ' + _BASE_DISABLE
_DISABLE_MSG = 'Connection.disable_table() was called, but ' + _BASE_DISABLE
_IS_ENABLED_MSG = ('Connection.is_table_enabled() was called, but ' +
_BASE_DISABLE)
_ENABLE_TMPL = 'Connection.enable_table(%r) was called, but ' + _BASE_DISABLE
_DISABLE_TMPL = 'Connection.disable_table(%r) was called, but ' + _BASE_DISABLE
_IS_ENABLED_TMPL = ('Connection.is_table_enabled(%r) was called, but ' +
_BASE_DISABLE)
_COMPACT_TMPL = ('Connection.compact_table(%r, major=%r) was called, but the '
'Cloud Bigtable API does not support compacting a table.')

This comment has been minimized.

Copy link
@mbrukman

mbrukman Jul 19, 2016

Contributor

I would say:

Cloud Bigtable handles table compactions automatically and does not expose an API for it.

The way it's written suggests that Cloud Bigtable does not actually know how to compact tables.



def _get_instance(timeout=None):
Expand Down Expand Up @@ -382,7 +384,8 @@ def delete_table(self, name, disable=False):
name = self._table_name(name)
_LowLevelTable(name, self._instance).delete()

def enable_table(self, name):
@staticmethod
def enable_table(name):
"""Enable the specified table.
.. warning::
Expand All @@ -393,9 +396,10 @@ def enable_table(self, name):
:type name: str
:param name: The name of the table to be enabled.
"""
_WARN(_ENABLE_MSG)
_WARN(_ENABLE_TMPL % (name,))

def disable_table(self, name):
@staticmethod
def disable_table(name):
"""Disable the specified table.
.. warning::
Expand All @@ -406,9 +410,10 @@ def disable_table(self, name):
:type name: str
:param name: The name of the table to be disabled.
"""
_WARN(_DISABLE_MSG)
_WARN(_DISABLE_TMPL % (name,))

def is_table_enabled(self, name):
@staticmethod
def is_table_enabled(name):
"""Return whether the specified table is enabled.
.. warning::
Expand All @@ -423,22 +428,25 @@ def is_table_enabled(self, name):
:rtype: bool
:returns: The value :data:`True` always.
"""
_WARN(_IS_ENABLED_MSG)
_WARN(_IS_ENABLED_TMPL % (name,))
return True

def compact_table(self, name, major=False):
@staticmethod
def compact_table(name, major=False):
"""Compact the specified table.
.. warning::
Cloud Bigtable does not support compacting a table, so this

This comment has been minimized.

Copy link
@mbrukman

mbrukman Jul 19, 2016

Contributor

Similar comment as above: Cloud Bigtable supports table compactions, it just doesn't expose an API for that feature.

method does not work. It is provided simply for compatibility.
method does nothing. It is provided simply for compatibility.
:type name: str
:param name: The name of the table to compact.
:raises: :class:`NotImplementedError <exceptions.NotImplementedError>`
always
:type major: bool
:param major: Whether to perform a major compaction.
"""
raise NotImplementedError('The Cloud Bigtable API does not support '
'compacting a table.')
_WARN(_COMPACT_TMPL % (name, major))


def _parse_family_option(option):
Expand Down
22 changes: 16 additions & 6 deletions gcloud/bigtable/happybase/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ def mock_warn(msg):
with _Monkey(MUT, _WARN=mock_warn):
connection.enable_table(name)

self.assertEqual(len(warned), 1)
self.assertEqual(warned, [MUT._ENABLE_TMPL % (name,)])

def test_disable_table(self):
from gcloud._testing import _Monkey
Expand All @@ -523,7 +523,7 @@ def mock_warn(msg):
with _Monkey(MUT, _WARN=mock_warn):
connection.disable_table(name)

self.assertEqual(len(warned), 1)
self.assertEqual(warned, [MUT._DISABLE_TMPL % (name,)])

def test_is_table_enabled(self):
from gcloud._testing import _Monkey
Expand All @@ -543,16 +543,26 @@ def mock_warn(msg):
result = connection.is_table_enabled(name)

self.assertTrue(result)
self.assertEqual(len(warned), 1)
self.assertEqual(warned, [MUT._IS_ENABLED_TMPL % (name,)])

def test_compact_table(self):
from gcloud._testing import _Monkey
from gcloud.bigtable.happybase import connection as MUT

instance = _Instance() # Avoid implicit environ check.
connection = self._makeOne(autoconnect=False, instance=instance)

name = 'table-name'
major = True
with self.assertRaises(NotImplementedError):
connection.compact_table(name, major=major)

warned = []

def mock_warn(msg):
warned.append(msg)

with _Monkey(MUT, _WARN=mock_warn):
connection.compact_table(name)

self.assertEqual(warned, [MUT._COMPACT_TMPL % (name, False)])


class Test__parse_family_option(unittest2.TestCase):
Expand Down

0 comments on commit 699c027

Please sign in to comment.