Simplifying Table.list_column_families.

Also removing ColumnFamily.from_pb in the process.

gcloud_bigtable/column_family.py

@@ -174,36 +174,6 @@ def __init__(self, column_family_id, table, gc_rule=None):
         self._table = table
         self.gc_rule = gc_rule
 
-    @classmethod
-    def from_pb(cls, column_family_pb, table):
-        """Creates a column family instance from a protobuf.
-
-        .. NOTE::
-
-            We ignore ``gc_expression`` in ``column_family_pb`` since our
-            helper classes don't set this value. This means that if another
-            client has created the column family being parsed, some
-            information from the protobuf may be unused.
-
-        :type column_family_pb: :class:`data_pb2.ColumnFamily`
-        :param column_family_pb: A column family protobuf object.
-
-        :type table: :class:`.table.Table`
-        :param table: The table that owns the column_family.
-
-        :rtype: :class:`ColumnFamily`
-        :returns: The column family parsed from the protobuf response.
-        :raises: :class:`ValueError` if one of the column family name does not
-                 begin with the table name
-        """
-        before, column_family_id = column_family_pb.name.split(
-            table.name + '/columnFamilies/', 1)
-        if before != '':
-            raise ValueError('Column family name %s not of expected format' % (
-                column_family_pb.name,))
-        gc_rule = _gc_rule_from_pb(column_family_pb.gc_rule)
-        return cls(column_family_id, table, gc_rule=gc_rule)
-
     @property
     def table(self):
         """Getter for column family's table.

gcloud_bigtable/table.py

@@ -19,6 +19,7 @@
     bigtable_table_service_messages_pb2 as messages_pb2)
 from gcloud_bigtable._helpers import make_stub
 from gcloud_bigtable.column_family import ColumnFamily
+from gcloud_bigtable.column_family import _gc_rule_from_pb
 from gcloud_bigtable.constants import TABLE_ADMIN_HOST
 from gcloud_bigtable.constants import TABLE_ADMIN_PORT
 from gcloud_bigtable.constants import TABLE_STUB_FACTORY
@@ -226,6 +227,9 @@ def list_column_families(self, timeout_seconds=None):
         :rtype: dictionary with string as keys and
                 :class:`.column_family.ColumnFamily` as values
         :returns: List of column families attached to this table.
+        :raises: :class:`ValueError` if the column family name from the
+                 response does not agree with the computed name from the
+                 column family ID.
         """
         request_pb = messages_pb2.GetTableRequest(name=self.name)
         stub = make_stub(self.client, TABLE_STUB_FACTORY,
@@ -236,5 +240,14 @@ def list_column_families(self, timeout_seconds=None):
             # We expect a `._generated.bigtable_table_data_pb2.Table`
             table_pb = response.result()
 
-        return {name: ColumnFamily.from_pb(value, self)
-                for name, value in table_pb.column_families.items()}
+        result = {}
+        for column_family_id, value_pb in table_pb.column_families.items():
+            gc_rule = _gc_rule_from_pb(value_pb.gc_rule)
+            column_family = self.column_family(column_family_id,
+                                               gc_rule=gc_rule)
+            if column_family.name != value_pb.name:
+                raise ValueError('Column family name %s does not agree with '
+                                 'name from request: %s.' % (
+                                     column_family.name, value_pb.name))
+            result[column_family_id] = column_family
+        return result

gcloud_bigtable/test_column_family.py

@@ -301,45 +301,6 @@ def test_constructor(self):
         self.assertTrue(column_family._table is table)
         self.assertTrue(column_family.gc_rule is gc_rule)
 
-    def test_from_pb_success(self):
-        from gcloud_bigtable._generated import (
-            bigtable_table_data_pb2 as data_pb2)
-
-        klass = self._getTargetClass()
-        table_name = 'table_name'
-        table = _Table(table_name)
-        column_family_name = table_name + '/columnFamilies/' + COLUMN_FAMILY_ID
-        column_family_pb = data_pb2.ColumnFamily(name=column_family_name)
-        column_family = klass.from_pb(column_family_pb, table)
-        self.assertTrue(isinstance(column_family, klass))
-        self.assertEqual(column_family.column_family_id, COLUMN_FAMILY_ID)
-        self.assertEqual(column_family.gc_rule, None)
-
-    def test_from_pb_failure(self):
-        from gcloud_bigtable._generated import (
-            bigtable_table_data_pb2 as data_pb2)
-
-        klass = self._getTargetClass()
-        table_name = 'table_name'
-        table = _Table(table_name)
-        column_family_name = 'does-not-start-with-table-name'
-        column_family_pb = data_pb2.ColumnFamily(name=column_family_name)
-        with self.assertRaises(ValueError):
-            klass.from_pb(column_family_pb, table)
-
-    def test_from_pb_failure_bad_before(self):
-        from gcloud_bigtable._generated import (
-            bigtable_table_data_pb2 as data_pb2)
-
-        klass = self._getTargetClass()
-        table_name = 'table_name'
-        table = _Table(table_name)
-        column_family_name = ('nonempty-section-before' + table_name +
-                              '/columnFamilies/' + COLUMN_FAMILY_ID)
-        column_family_pb = data_pb2.ColumnFamily(name=column_family_name)
-        with self.assertRaises(ValueError):
-            klass.from_pb(column_family_pb, table)
-
     def test_table_getter(self):
         table = object()
         column_family = self._makeOne(COLUMN_FAMILY_ID, table)

gcloud_bigtable/test_table.py

@@ -229,7 +229,7 @@ def result_method(client):
                                       PROJECT_ID,
                                       timeout_seconds=timeout_seconds)
 
-    def test_list_column_families(self):
+    def _list_column_families_helper(self, column_family_name=None):
         from gcloud_bigtable._generated import (
             bigtable_table_data_pb2 as data_pb2)
         from gcloud_bigtable._generated import (
@@ -243,7 +243,9 @@ def test_list_column_families(self):
 
         # Create response_pb
         column_family_id = 'foo'
-        column_family_name = table_name + '/columnFamilies/' + column_family_id
+        if column_family_name is None:
+            column_family_name = (table_name + '/columnFamilies/' +
+                                  column_family_id)
         column_family = data_pb2.ColumnFamily(name=column_family_name)
         response_pb = data_pb2.Table(
             column_families={column_family_id: column_family},
@@ -269,6 +271,15 @@ def result_method(client):
                                       PROJECT_ID,
                                       timeout_seconds=timeout_seconds)
 
+    def test_list_column_families(self):
+        self._list_column_families_helper()
+
+    def test_list_column_families_failure(self):
+        column_family_name = 'not-the-right-format'
+        with self.assertRaises(ValueError):
+            self._list_column_families_helper(
+                column_family_name=column_family_name)
+
 
 class _Cluster(object):
 

tox.ini

@@ -1,6 +1,6 @@
 [tox]
 envlist =
-    py27,cover,lint
+    py27,cover,lint,docs
 
 [testenv]
 commands =