Checking families in HappyBase Connection.create_table.

Also retro-fitting tests to pass in Python 3.4.

gcloud_bigtable/_helpers.py

@@ -239,24 +239,6 @@ def _microseconds_to_timestamp(microseconds):
     return EPOCH + datetime.timedelta(microseconds=microseconds)
 
 
-def _to_bytes(value):
-    """Converts a value to bytes (or checks that it already is).
-
-    :type value: bytes
-    :param value: The value to ensure is converted to bytes.
-
-    :rtype: bytes
-    :returns: The ``value`` as bytes.
-    :raises: :class:`TypeError <exceptions.TypeError>` if the ``value`` is not
-             bytes or string.
-    """
-    if isinstance(value, six.text_type):
-        value = value.encode('utf-8')
-    if not isinstance(value, bytes):
-        raise TypeError('Row key must be bytes.')
-    return value
-
-
 def _set_certs():
     """Sets the cached root certificates locally."""
     with open(SSL_CERT_FILE, mode='rb') as file_obj:
@@ -353,3 +335,34 @@ def _parse_family_pb(family_pb):
             cells.append(val_pair)
 
     return family_pb.name, result
+
+
+def _to_bytes(value, encoding='ascii'):
+    """Converts a string value to bytes, if necessary.
+
+    Unfortunately, ``six.b`` is insufficient for this task since in
+    Python2 it does not modify ``unicode`` objects.
+
+    :type value: str / bytes or unicode
+    :param value: The string/bytes value to be converted.
+
+    :type encoding: str
+    :param encoding: The encoding to use to convert unicode to bytes. Defaults
+                     to "ascii", which will not allow any characters from
+                     ordinals larger than 127. Other useful values are
+                     "latin-1", which which will only allows byte ordinals
+                     (up to 255) and "utf-8", which will encode any unicode
+                     that needs to be.
+
+    :rtype: str / bytes
+    :returns: The original value converted to bytes (if unicode) or as passed
+              in if it started out as bytes.
+    :raises: :class:`TypeError <exceptions.TypeError>` if the value
+             could not be converted to bytes.
+    """
+    result = (value.encode(encoding)
+              if isinstance(value, six.text_type) else value)
+    if isinstance(result, six.binary_type):
+        return result
+    else:
+        raise TypeError('%r could not be converted to bytes' % (value,))

gcloud_bigtable/happybase/connection.py

@@ -364,7 +364,19 @@ def create_table(self, name, families):
                          * :class:`.GarbageCollectionRule`
                          * :class:`.GarbageCollectionRuleUnion`
                          * :class:`.GarbageCollectionRuleIntersection`
+
+        :raises: :class:`TypeError <exceptions.TypeError>` if ``families`` is
+                 not a dictionary,
+                 :class:`ValueError <exceptions.ValueError>` if ``families``
+                 has no entries
         """
+        if not isinstance(families, dict):
+            raise TypeError('families arg must be a dictionary')
+
+        if not families:
+            raise ValueError('Cannot create table %r (no column '
+                             'families specified)' % (name,))
+
         # Parse all keys before making any API requests.
         gc_rule_dict = {}
         for column_family_name, option in families.items():

gcloud_bigtable/happybase/pool.py

@@ -15,7 +15,6 @@
 """Google Cloud Bigtable HappyBase pool module."""
 
 
-from Queue import LifoQueue
 import six
 import threading
 
@@ -63,7 +62,7 @@ def __init__(self, size, **kwargs):
             raise ValueError('Pool size must be positive')
 
         self._lock = threading.Lock()
-        self._queue = LifoQueue(maxsize=size)
+        self._queue = six.moves.queue.LifoQueue(maxsize=size)
         self._thread_connections = threading.local()
 
         connection_kwargs = kwargs
@@ -72,7 +71,7 @@ def __init__(self, size, **kwargs):
             connection_kwargs['cluster'] = _get_cluster(
                 timeout=kwargs.get('timeout'))
 
-        for _ in xrange(size):
+        for _ in six.moves.range(size):
             connection = Connection(**connection_kwargs)
             self._queue.put(connection)
 

gcloud_bigtable/happybase/test_connection.py

@@ -424,6 +424,24 @@ def test_create_table(self):
         self.assertEqual(col_fam_instances[1].gc_rule, mock_gc_rule)
         self.assertEqual(col_fam_instances[1].create_calls, 1)
 
+    def test_create_table_bad_type(self):
+        cluster = _Cluster()  # Avoid implicit environ check.
+        connection = self._makeOne(autoconnect=False, cluster=cluster)
+
+        name = 'table-name'
+        families = None
+        with self.assertRaises(TypeError):
+            connection.create_table(name, families)
+
+    def test_create_table_bad_value(self):
+        cluster = _Cluster()  # Avoid implicit environ check.
+        connection = self._makeOne(autoconnect=False, cluster=cluster)
+
+        name = 'table-name'
+        families = {}
+        with self.assertRaises(ValueError):
+            connection.create_table(name, families)
+
     def test_delete_table(self):
         from gcloud_bigtable._testing import _Monkey
         from gcloud_bigtable.happybase import connection as MUT

gcloud_bigtable/happybase/test_pool.py

@@ -26,7 +26,7 @@ def _makeOne(self, *args, **kwargs):
         return self._getTargetClass()(*args, **kwargs)
 
     def test_constructor_defaults(self):
-        from Queue import LifoQueue
+        import six
         import threading
         from gcloud_bigtable.happybase.connection import Connection
 
@@ -41,7 +41,7 @@ def test_constructor_defaults(self):
         self.assertEqual(pool._thread_connections.__dict__, {})
 
         queue = pool._queue
-        self.assertTrue(isinstance(queue, LifoQueue))
+        self.assertTrue(isinstance(queue, six.moves.queue.LifoQueue))
         self.assertTrue(queue.full())
         self.assertEqual(queue.maxsize, size)
         for connection in queue.queue:

gcloud_bigtable/happybase/test_table.py

@@ -515,8 +515,8 @@ def test_row_with_results(self):
         fake_pair = object()
         mock_cells_to_pairs = _MockCalled([fake_pair])
 
-        col_fam = 'cf1'
-        qual = 'qual'
+        col_fam = u'cf1'
+        qual = b'qual'
         fake_cells = object()
         partial_row._cells = {col_fam: {qual: fake_cells}}
         include_timestamp = object()
@@ -525,7 +525,7 @@ def test_row_with_results(self):
             result = table.row(row_key, include_timestamp=include_timestamp)
 
         # The results come from _cells_to_pairs.
-        expected_result = {col_fam + ':' + qual: fake_pair}
+        expected_result = {col_fam.encode('ascii') + b':' + qual: fake_pair}
         self.assertEqual(result, expected_result)
 
         read_row_args = (row_key,)

gcloud_bigtable/row_data.py

@@ -19,6 +19,7 @@
 import six
 
 from gcloud_bigtable._helpers import _microseconds_to_timestamp
+from gcloud_bigtable._helpers import _to_bytes
 
 
 class Cell(object):
@@ -97,7 +98,8 @@ def to_dict(self):
         result = {}
         for column_family_id, columns in six.iteritems(self._cells):
             for column_qual, cells in six.iteritems(columns):
-                key = column_family_id + ':' + column_qual
+                key = (_to_bytes(column_family_id) + b':' +
+                       _to_bytes(column_qual))
                 result[key] = cells
         return result
 

gcloud_bigtable/test__helpers.py

@@ -305,33 +305,6 @@ def test_it(self):
         self.assertEqual(timestamp, self._callFUT(microseconds))
 
 
-class Test__to_bytes(unittest2.TestCase):
-
-    def _callFUT(self, value):
-        from gcloud_bigtable._helpers import _to_bytes
-        return _to_bytes(value)
-
-    def test_with_bytes(self):
-        value = b'value'
-        result = self._callFUT(value)
-        # Only necessary in Py2
-        self.assertEqual(type(result), type(value))
-        self.assertEqual(result, value)
-
-    def test_with_string(self):
-        value = u'value'
-        bytes_val = b'value'
-        result = self._callFUT(value)
-        # Only necessary in Py2
-        self.assertNotEqual(type(result), type(value))
-        self.assertEqual(result, bytes_val)
-
-    def test_with_non_string_types(self):
-        value = object()
-        with self.assertRaises(TypeError):
-            self._callFUT(value)
-
-
 class Test__set_certs(unittest2.TestCase):
 
     def _callFUT(self):
@@ -528,3 +501,30 @@ def test_it(self):
             ],
         )
         self.assertEqual(expected_output, self._callFUT(sample_input))
+
+
+class Test__to_bytes(unittest2.TestCase):
+
+    def _callFUT(self, *args, **kwargs):
+        from gcloud_bigtable._helpers import _to_bytes
+        return _to_bytes(*args, **kwargs)
+
+    def test_with_bytes(self):
+        value = b'bytes-val'
+        self.assertEqual(self._callFUT(value), value)
+
+    def test_with_unicode(self):
+        value = u'string-val'
+        encoded_value = b'string-val'
+        self.assertEqual(self._callFUT(value), encoded_value)
+
+    def test_unicode_non_ascii(self):
+        value = u'\u2013'  # Long hyphen
+        encoded_value = b'\xe2\x80\x93'
+        self.assertRaises(UnicodeEncodeError, self._callFUT, value)
+        self.assertEqual(self._callFUT(value, encoding='utf-8'),
+                         encoded_value)
+
+    def test_with_nonstring_type(self):
+        value = object()
+        self.assertRaises(TypeError, self._callFUT, value)

gcloud_bigtable/test_row.py

@@ -205,7 +205,7 @@ def test_set_cell_with_non_bytes_value(self):
         row = self._makeOne(ROW_KEY, table)
         value = object()  # Not bytes
         with self.assertRaises(TypeError):
-            row.set_cell(None, None, value)
+            row.set_cell(COLUMN_FAMILY_ID, COLUMN, value)
 
     def test_set_cell_with_non_null_timestamp(self):
         import datetime

gcloud_bigtable/test_row_data.py

@@ -155,9 +155,9 @@ def test_to_dict(self):
         }
 
         result = partial_row_data.to_dict()
-        col1 = family_name1 + b':' + qual1
-        col2 = family_name1 + b':' + qual2
-        col3 = family_name2 + b':' + qual3
+        col1 = family_name1.encode('ascii') + b':' + qual1
+        col2 = family_name1.encode('ascii') + b':' + qual2
+        col3 = family_name2.encode('ascii') + b':' + qual3
         expected_result = {
             col1: cell1,
             col2: cell2,
@@ -509,4 +509,4 @@ def cancel(self):
         self.cancel_calls += 1
 
     def next(self):
-        return self.iter_values.next()
+        return next(self.iter_values)

system_tests/run_happybase.py

@@ -97,6 +97,15 @@ def test_create_and_delete_table(self):
         connection.delete_table(ALT_TABLE_NAME, disable=True)
         self.assertFalse(ALT_TABLE_NAME in connection.tables())
 
+    def test_create_table_failure(self):
+        connection = get_connection()
+
+        self.assertFalse(ALT_TABLE_NAME in connection.tables())
+        empty_families = {}
+        with self.assertRaises(ValueError):
+            connection.create_table(ALT_TABLE_NAME, empty_families)
+        self.assertFalse(ALT_TABLE_NAME in connection.tables())
+
 
 class BaseTableTest(unittest2.TestCase):