From 6bd2442dcb9bc0049d2a74800eb53f8dff00d719 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Tue, 19 Jul 2016 11:39:09 -0700 Subject: [PATCH 1/3] Making HappyBase enabled/disabled checks into no-ops. Fixes #1989. --- gcloud/bigtable/happybase/connection.py | 40 +++++++++++-------- gcloud/bigtable/happybase/test_connection.py | 42 ++++++++++++++++++-- 2 files changed, 61 insertions(+), 21 deletions(-) diff --git a/gcloud/bigtable/happybase/connection.py b/gcloud/bigtable/happybase/connection.py index ebea84e93998..fa9d541c02ef 100644 --- a/gcloud/bigtable/happybase/connection.py +++ b/gcloud/bigtable/happybase/connection.py @@ -49,9 +49,13 @@ _LEGACY_ARGS = frozenset(('host', 'port', 'compat', 'transport', 'protocol')) _WARN = warnings.warn +_BASE_DISABLE = 'Cloud Bigtable has no concept of enabled / disabled tables.' _DISABLE_DELETE_MSG = ('The disable argument should not be used in ' - 'delete_table(). Cloud Bigtable has no concept ' - 'of enabled / disabled tables.') + '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) def _get_instance(timeout=None): @@ -384,13 +388,12 @@ def enable_table(self, name): .. warning:: Cloud Bigtable has no concept of enabled / disabled tables so this - method does not work. It is provided simply for compatibility. + method does nothing. It is provided simply for compatibility. - :raises: :class:`NotImplementedError ` - always + :type name: str + :param name: The name of the table to be enabled. """ - raise NotImplementedError('The Cloud Bigtable API has no concept of ' - 'enabled or disabled tables.') + _WARN(_ENABLE_MSG) def disable_table(self, name): """Disable the specified table. @@ -398,13 +401,12 @@ def disable_table(self, name): .. warning:: Cloud Bigtable has no concept of enabled / disabled tables so this - method does not work. It is provided simply for compatibility. + method does nothing. It is provided simply for compatibility. - :raises: :class:`NotImplementedError ` - always + :type name: str + :param name: The name of the table to be disabled. """ - raise NotImplementedError('The Cloud Bigtable API has no concept of ' - 'enabled or disabled tables.') + _WARN(_DISABLE_MSG) def is_table_enabled(self, name): """Return whether the specified table is enabled. @@ -412,13 +414,17 @@ def is_table_enabled(self, name): .. warning:: Cloud Bigtable has no concept of enabled / disabled tables so this - method does not work. It is provided simply for compatibility. + method always returns :data:`True`. It is provided simply for + compatibility. - :raises: :class:`NotImplementedError ` - always + :type name: str + :param name: The name of the table to check enabled / disabled status. + + :rtype: bool + :returns: The value :data:`True` always. """ - raise NotImplementedError('The Cloud Bigtable API has no concept of ' - 'enabled or disabled tables.') + _WARN(_IS_ENABLED_MSG) + return True def compact_table(self, name, major=False): """Compact the specified table. diff --git a/gcloud/bigtable/happybase/test_connection.py b/gcloud/bigtable/happybase/test_connection.py index 6236539db71f..8b2f65ecf064 100644 --- a/gcloud/bigtable/happybase/test_connection.py +++ b/gcloud/bigtable/happybase/test_connection.py @@ -488,28 +488,62 @@ def mock_warn(msg): self.assertEqual(warned, [MUT._DISABLE_DELETE_MSG]) def test_enable_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' - with self.assertRaises(NotImplementedError): + + warned = [] + + def mock_warn(msg): + warned.append(msg) + + with _Monkey(MUT, _WARN=mock_warn): connection.enable_table(name) + self.assertEqual(len(warned), 1) + def test_disable_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' - with self.assertRaises(NotImplementedError): + + warned = [] + + def mock_warn(msg): + warned.append(msg) + + with _Monkey(MUT, _WARN=mock_warn): connection.disable_table(name) + self.assertEqual(len(warned), 1) + def test_is_table_enabled(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' - with self.assertRaises(NotImplementedError): - connection.is_table_enabled(name) + + warned = [] + + def mock_warn(msg): + warned.append(msg) + + with _Monkey(MUT, _WARN=mock_warn): + result = connection.is_table_enabled(name) + + self.assertTrue(result) + self.assertEqual(len(warned), 1) def test_compact_table(self): instance = _Instance() # Avoid implicit environ check. From 699c027e828e0f3f7e37ca8f5efd801914cdb944 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Tue, 19 Jul 2016 12:34:45 -0700 Subject: [PATCH 2/3] Making HappyBase compact_table() a no-op. Also fixing some lint related errors (not using arguments and not using self). --- gcloud/bigtable/happybase/connection.py | 40 ++++++++++++-------- gcloud/bigtable/happybase/test_connection.py | 22 ++++++++--- 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/gcloud/bigtable/happybase/connection.py b/gcloud/bigtable/happybase/connection.py index fa9d541c02ef..19059b0239cf 100644 --- a/gcloud/bigtable/happybase/connection.py +++ b/gcloud/bigtable/happybase/connection.py @@ -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.') def _get_instance(timeout=None): @@ -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:: @@ -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:: @@ -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:: @@ -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 - 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 ` - 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): diff --git a/gcloud/bigtable/happybase/test_connection.py b/gcloud/bigtable/happybase/test_connection.py index 8b2f65ecf064..f70c69eaaea5 100644 --- a/gcloud/bigtable/happybase/test_connection.py +++ b/gcloud/bigtable/happybase/test_connection.py @@ -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 @@ -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 @@ -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): From 0cb6e6071a562b9083304f39e3be0f63f98f33f5 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Tue, 19 Jul 2016 12:56:10 -0700 Subject: [PATCH 3/3] Fixing language about HappyBase table compaction. --- gcloud/bigtable/happybase/connection.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/gcloud/bigtable/happybase/connection.py b/gcloud/bigtable/happybase/connection.py index 19059b0239cf..3780763cd033 100644 --- a/gcloud/bigtable/happybase/connection.py +++ b/gcloud/bigtable/happybase/connection.py @@ -57,7 +57,8 @@ _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.') + 'Cloud Bigtable API handles table compactions automatically ' + 'and does not expose an API for it.') def _get_instance(timeout=None): @@ -437,8 +438,9 @@ def compact_table(name, major=False): .. warning:: - Cloud Bigtable does not support compacting a table, so this - method does nothing. It is provided simply for compatibility. + Cloud Bigtable supports table compactions, it just doesn't expose + an API for that feature, so this method does nothing. It is + provided simply for compatibility. :type name: str :param name: The name of the table to compact.