Raising errors in code paths that Cloud Bigtable can't support.

Also fixing some broken tests caused by changes made to get
HappyBase system tests passing (allowing use of exclusive
timestamp range ends, which was previously assumed to be
inclusive, since they are inclusive on deletes in HBase).

gcloud_bigtable/happybase/batch.py

@@ -223,11 +223,18 @@ def _delete_columns(self, columns, row_object):
 
         :type row_object: :class:`Row <gcloud_bigtable.row.Row>`
         :param row_object: The row which will hold the delete mutations.
+
+        :raises: :class:`ValueError <exceptions.ValueError>` if the delete
+                 timestamp range is set on the current batch, but a
+                 column family delete is attempted.
         """
         column_pairs = _get_column_pairs(columns)
         for column_family_id, column_qualifier in column_pairs:
             if column_qualifier is None:
-                # NOTE: time_range not part of `DeleteFromFamily`
+                if self._delete_range is not None:
+                    raise ValueError('The Cloud Bigtable API does not support '
+                                     'adding a timestamp to '
+                                     '"DeleteFromFamily" ')
                 row_object.delete_cells(column_family_id,
                                         columns=row_object.ALL_COLUMNS)
             else:
@@ -257,7 +264,8 @@ def delete(self, row, columns=None, wal=_WAL_SENTINEL):
                     Write Ahead Log.
 
         :raises: :class:`ValueError <exceptions.ValueError>` if ``wal``
-                 is used.
+                 is used, or if if the delete timestamp range is set on the
+                 current batch, but a full row delete is attempted.
         """
         if wal is not _WAL_SENTINEL:
             raise ValueError('The wal argument cannot be used with '
@@ -267,8 +275,11 @@ def delete(self, row, columns=None, wal=_WAL_SENTINEL):
 
         if columns is None:
             # Delete entire row.
+            if self._delete_range is not None:
+                raise ValueError('The Cloud Bigtable API does not support '
+                                 'adding a timestamp to "DeleteFromRow" '
+                                 'mutations')
             row_object.delete()
-            # NOTE: time_range not part of `DeleteFromRow`
             self._mutation_count += 1
         else:
             self._delete_columns(columns, row_object)

gcloud_bigtable/happybase/table.py

@@ -622,10 +622,13 @@ def scan(self, row_start=None, row_stop=None, row_prefix=None,
                           * an entire column family: ``fam`` or ``fam:``
                           * an single column: ``fam:col``
 
-        :type filter: str
-        :param filter: (Optional) An HBase filter string. See
+        :type filter: :class:`RowFilter`, :class:`RowFilterChain`,
+                      :class:`RowFilterUnion` or :class:`ConditionalRowFilter`
+        :param filter: (Optional) An additional filter (beyond column and
+                       row range filters supported here). HappyBase / HBase
+                       users will have used this as an HBase filter string. See
                        http://hbase.apache.org/0.94/book/thrift.html
-                       for more details.
+                       for more details on those filters.
 
         :type timestamp: int
         :param timestamp: (Optional) Timestamp (in milliseconds since the
@@ -661,7 +664,9 @@ def scan(self, row_start=None, row_stop=None, row_prefix=None,
                  or ``scan_batching`` are used, or if ``limit`` is set but
                  non-positive, or if row prefix is used with row start/stop,
                  :class:`NotImplementedError <exceptions.NotImplementedError>`
-                 temporarily until the method is implemented.
+                 temporarily until the method is implemented,
+                 :class:`TypeError <exceptions.TypeError>` if a string
+                 ``filter`` is used.
         """
         if batch_size is not _DEFAULT_BATCH_SIZE:
             raise ValueError('Batch size cannot be set for gcloud '
@@ -682,6 +687,13 @@ def scan(self, row_start=None, row_stop=None, row_prefix=None,
             row_stop = _string_successor(row_prefix)
 
         filters = []
+        if isinstance(filter, six.string_types):
+            raise TypeError('HBase filter strings not supported by Cloud '
+                            'Bigtable. RowFilter\'s from row module may be '
+                            'used instead.')
+        elif filter is not None:
+            filters.append(filter)
+
         if columns is not None:
             filters.append(_columns_filter_helper(columns))
         # versions == 1 since we only want the latest.

gcloud_bigtable/happybase/test_batch.py

@@ -302,9 +302,8 @@ def _try_send(self):
         self.assertEqual(batch._mutation_count, 0)
         self.assertEqual(batch.try_send_calls, 1)
 
-    def test__delete_columns(self):
+    def _delete_columns_test_helper(self, time_range=None):
         table = object()
-        time_range = object()
         batch = self._makeOne(table)
         batch._delete_range = time_range
 
@@ -326,6 +325,14 @@ def test__delete_columns(self):
         self.assertEqual(row_object.delete_cells_calls,
                          [(fam_deleted_args, fam_deleted_kwargs)])
 
+    def test__delete_columns(self):
+        self._delete_columns_test_helper()
+
+    def test__delete_columns_w_time_and_col_fam(self):
+        time_range = object()
+        with self.assertRaises(ValueError):
+            self._delete_columns_test_helper(time_range=time_range)
+
     def test_delete_bad_wal(self):
         from gcloud_bigtable.happybase.batch import _WAL_SENTINEL
 
@@ -353,6 +360,21 @@ def test_delete_entire_row(self):
         self.assertEqual(row.deletes, 1)
         self.assertEqual(batch._mutation_count, 1)
 
+    def test_delete_entire_row_with_ts(self):
+        table = object()
+        batch = self._makeOne(table)
+        batch._delete_range = object()
+
+        row_key = 'row-key'
+        batch._row_map[row_key] = row = _MockRow()
+
+        self.assertEqual(row.deletes, 0)
+        self.assertEqual(batch._mutation_count, 0)
+        with self.assertRaises(ValueError):
+            batch.delete(row_key, columns=None)
+        self.assertEqual(row.deletes, 0)
+        self.assertEqual(batch._mutation_count, 0)
+
     def test_delete_call_try_send(self):
         klass = self._getTargetClass()
 

gcloud_bigtable/happybase/test_table.py

@@ -151,11 +151,11 @@ def test_success(self):
         from gcloud_bigtable.row import TimestampRange
 
         timestamp = 1441928298571
-        next_ts = _microseconds_to_timestamp(1000 * (timestamp + 1))
+        ts_dt = _microseconds_to_timestamp(1000 * timestamp)
         result = self._callFUT(timestamp=timestamp)
         self.assertTrue(isinstance(result, TimestampRange))
         self.assertEqual(result.start, None)
-        self.assertEqual(result.end, next_ts)
+        self.assertEqual(result.end, ts_dt)
 
 
 class Test__cells_to_pairs(unittest2.TestCase):
@@ -282,8 +282,8 @@ def test_with_timestamp(self):
         time_range = range_filter.timestamp_range_filter
         self.assertTrue(isinstance(time_range, TimestampRange))
         self.assertEqual(time_range.start, None)
-        next_ts = _microseconds_to_timestamp(1000 * (timestamp + 1))
-        self.assertEqual(time_range.end, next_ts)
+        ts_dt = _microseconds_to_timestamp(1000 * timestamp)
+        self.assertEqual(time_range.end, ts_dt)
 
     def test_with_all_options(self):
         versions = 11
@@ -840,8 +840,15 @@ def test_scan_with_row_prefix_and_row_start(self):
         with self.assertRaises(ValueError):
             list(table.scan(row_prefix='a', row_stop='abc'))
 
+    def test_scan_with_string_filter(self):
+        name = 'table-name'
+        connection = None
+        table = self._makeOne(name, connection)
+        with self.assertRaises(TypeError):
+            list(table.scan(filter='some-string'))
+
     def _scan_test_helper(self, row_start=None, row_stop=None, row_prefix=None,
-                          columns=None, timestamp=None,
+                          columns=None, filter_=None, timestamp=None,
                           include_timestamp=False, limit=None, rr_result=None,
                           expected_result=None):
         import types
@@ -866,7 +873,7 @@ def _scan_test_helper(self, row_start=None, row_stop=None, row_prefix=None,
                      _columns_filter_helper=mock_columns_filter_helper):
             result = table.scan(row_start=row_start, row_stop=row_stop,
                                 row_prefix=row_prefix, columns=columns,
-                                timestamp=timestamp,
+                                filter=filter_, timestamp=timestamp,
                                 include_timestamp=include_timestamp,
                                 limit=limit)
             self.assertTrue(isinstance(result, types.GeneratorType))
@@ -896,10 +903,11 @@ def _scan_test_helper(self, row_start=None, row_stop=None, row_prefix=None,
         else:
             mock_columns_filter_helper.check_called(self, [])
 
+        filters = []
+        if filter_ is not None:
+            filters.append(filter_)
         if columns:
-            filters = [fake_col_filter]
-        else:
-            filters = []
+            filters.append(fake_col_filter)
         expected_kwargs = {
             'filters': filters,
             'versions': 1,
@@ -920,6 +928,10 @@ def test_scan_with_row_prefix(self):
         row_prefix = 'row-prefi'
         self._scan_test_helper(row_prefix=row_prefix)
 
+    def test_scan_with_filter(self):
+        mock_filter = object()
+        self._scan_test_helper(filter_=mock_filter)
+
     def test_scan_with_no_results(self):
         limit = 1337
         timestamp = object()