YACHT-1021: SLI should not list partitions in non partitioned tables

ocadotechnology · Sep 14, 2018 · 7e4f230 · 7e4f230
1 parent 56cacaf
commit 7e4f230
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 15 deletions.
diff --git a/src/commons/big_query/big_query.py b/src/commons/big_query/big_query.py
@@ -193,7 +193,7 @@ def get_table(self, project_id, dataset_id, table_id, log_table=True):
 
         except HttpError as ex:
             if ex.resp.status == 404:
-                logging.warning(
+                logging.info(
                     "Table '%s' Not Found",
                     TableReference(project_id, dataset_id, table_id)
                 )

diff --git a/src/slo/x_days_sli/sli_table_exists_filter.py b/src/slo/x_days_sli/sli_table_exists_filter.py
@@ -25,7 +25,8 @@ def exists(self, table_reference):
                          table_reference)
             return False
 
-        if not table_reference.is_partition():
+        if not table_reference.is_partition() or \
+           not table_metadata.has_time_partitioning():
             logging.info("Non-partitioned table exist: %s", table_reference)
             return True
 

diff --git a/src/slo/x_days_sli/x_days_sli_service.py b/src/slo/x_days_sli/x_days_sli_service.py
@@ -21,19 +21,19 @@ def recalculate_sli(self):
 
         all_tables = self.querier.query(self.x_days)
         filtered_tables = [table for table in all_tables
-                           if self.__should_stay_as_SLI_violation(table)]
+                           if self.__should_stay_as_sli_violation(table)]
 
         logging.info("%s days SLI tables filtered from %s to %s", self.x_days,
                      len(all_tables), len(filtered_tables))
         self.streamer.stream(filtered_tables)
 
-    def __should_stay_as_SLI_violation(self, table):
+    def __should_stay_as_sli_violation(self, table):
         try:
-            exist = self.filter.exists(self.__create_table_reference(table))
-        except:
-            logging.error("An error occured while filtering table %s", table)
+            return self.filter.exists(self.__create_table_reference(table))
+        except Exception:
+            logging.exception("An error occurred while filtering table %s, "
+                              "still it will be streamed", table)
             return True
-        return exist
 
     @staticmethod
     def __create_table_reference(table):

diff --git a/tests/slo/x_days_sli/test_sli_table_exists_filter.py b/tests/slo/x_days_sli/test_sli_table_exists_filter.py
@@ -40,11 +40,10 @@ def test_should_return_true_for_existing_table(self):
     @patch('src.commons.big_query.big_query.BigQuery.__init__',
            Mock(return_value=None))
     @patch('src.commons.big_query.big_query.BigQuery.get_table',
-           Mock(return_value={'projectId': 'p', 'schema': {'fields': []}}))
+           Mock(return_value={'projectId': 'p', 'schema': {'fields': []},
+                              'timePartitioning': {'type': 'DAY'}}))
     @patch('src.commons.big_query.big_query.BigQuery.list_table_partitions',
            Mock(return_value=[]))
-    @patch('src.commons.big_query.big_query.BigQuery.list_table_partitions',
-           Mock(return_value=[{'partitionId': 'other_partition_id'}]))
     def test_should_return_false_for_not_existing_partition(self):
         # given
         table_reference = TableReference('p', 'd', 't', '20180808')
@@ -84,3 +83,19 @@ def test_should_return_false_when_there_is_no_schema(self):
 
         # then
         self.assertFalse(exists)
+
+    @patch('src.commons.big_query.big_query.BigQuery.__init__',
+           Mock(return_value=None))
+    @patch('src.commons.big_query.big_query.BigQuery.get_table',
+           Mock(return_value={'projectId': 'p', 'schema': {'fields': []}}))
+    @patch.object(BigQuery, 'list_table_partitions')
+    def test_should_not_list_partitions_in_non_partitioned_table(self, list_table_partitions):
+        # given
+        table_reference = TableReference('p', 'd', 't')
+
+        # when
+        exists = SLITableExistsFilter(BigQuery()).exists(table_reference)
+
+        # then
+        self.assertTrue(exists)
+        list_table_partitions.assert_not_called()
diff --git a/tests/slo/x_days_sli/test_x_days_sli_service.py b/tests/slo/x_days_sli/test_x_days_sli_service.py
@@ -11,10 +11,15 @@
 class TestXDaysSLIService(unittest.TestCase):
 
     @patch.object(SLIViewQuerier, 'query', return_value=[
-        {"projectId":"p1", "datasetId":"d1", "tableId":"t1", "partitionId":"part1"}
+        {"projectId": "p1", "datasetId": "d1", "tableId": "t1",
+         "partitionId": "part1"}
     ])
-    @patch.object(SLITableExistsFilter, 'exists', side_effect=Exception("An error"))
+    @patch.object(SLITableExistsFilter, 'exists',
+                  side_effect=Exception("An error"))
     @patch.object(SLIResultsStreamer, 'stream')
-    def test_table_that_caused_exception_should_not_be_filtered_out(self, stream, _, query):
+    def test_table_that_caused_exception_should_not_be_filtered_out(self,
+                                                                    stream,
+                                                                    _, _1):
         XDaysSLIService(3).recalculate_sli()
-        stream.assert_called_with([{"projectId":"p1", "datasetId":"d1", "tableId":"t1", "partitionId":"part1"}])
+        stream.assert_called_with([{"projectId": "p1", "datasetId": "d1",
+                                    "tableId": "t1", "partitionId": "part1"}])