fix: fix invalid timestamp handling in dashboard transformer (amundse…

…n-io#339) * fix: fix invalid timestamp handling in dashboard transformer Signed-off-by: feng-tao <[email protected]> * fix: fix ci Signed-off-by: feng-tao <[email protected]> * chore: fix rest_api_query to do basic check Signed-off-by: feng-tao <[email protected]> * Update setup.py Signed-off-by: Tao Feng <[email protected]>
brexhq · Aug 20, 2020 · 030ef49 · 030ef49
1 parent e14b33e
commit 030ef49
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 4 deletions.
diff --git a/databuilder/models/dashboard/dashboard_table.py b/databuilder/models/dashboard/dashboard_table.py
@@ -57,7 +57,7 @@ def create_next_relation(self) -> Union[Dict[str, Any], None]:
 
     def _create_relation_iterator(self) -> Optional[Iterator[Dict[str, Any]]]:
         for table_id in self._table_ids:
-            m = re.match('(\w+)://(\w+)\.(\w+)\/(\w+)', table_id)
+            m = re.match('([^./]+)://([^./]+)\.([^./]+)\/([^./]+)', table_id)
             if m:
                 yield {
                     RELATION_START_LABEL: DashboardMetadata.DASHBOARD_NODE_LABEL,

diff --git a/databuilder/rest_api/rest_api_query.py b/databuilder/rest_api/rest_api_query.py
@@ -153,7 +153,7 @@ def execute(self) -> Iterator[Dict[str, Any]]:  # noqa: C901
                 result_list: List[Any] = [match.value for match in self._jsonpath_expr.find(response_json)]
 
                 if not result_list:
-                    log_msg = 'No result from URL: {url}  , JSONPATH: {json_path} , response payload: {response}' \
+                    log_msg = 'No result from URL: {url}, JSONPATH: {json_path} , response payload: {response}' \
                         .format(url=self._url, json_path=self._json_path, response=response_json)
                     LOGGER.info(log_msg)
 
@@ -172,6 +172,9 @@ def execute(self) -> Iterator[Dict[str, Any]]:  # noqa: C901
                                                                 json_path_contains_or=self._json_path_contains_or)
 
                 for sub_record in sub_records:
+                    if not sub_record or len(sub_record) != len(self._field_names):
+                        # skip the record
+                        continue
                     record_dict = copy.deepcopy(record_dict)
                     for field_name in self._field_names:
                         record_dict[field_name] = sub_record.pop(0)

diff --git a/databuilder/transformer/timestamp_string_to_epoch.py b/databuilder/transformer/timestamp_string_to_epoch.py
@@ -34,7 +34,12 @@ def transform(self, record: Dict[str, Any]) -> Dict[str, Any]:
         if not timestamp_str:
             return record
 
-        utc_dt = datetime.strptime(timestamp_str, self._timestamp_format)
+        try:
+            utc_dt = datetime.strptime(timestamp_str, self._timestamp_format)
+        except ValueError:
+            # if the timestamp_str doesn't match format, no conversion, return initial result
+            record[self._field_name] = 0
+            return record
 
         record[self._field_name] = int((utc_dt - datetime(1970, 1, 1)).total_seconds())
         return record

diff --git a/setup.py b/setup.py
@@ -5,7 +5,7 @@
 from setuptools import setup, find_packages
 
 
-__version__ = '3.0.0'
+__version__ = '3.1.0'
 
 requirements_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'requirements.txt')
 with open(requirements_path) as requirements_file:

diff --git a/tests/unit/models/dashboard/test_dashboard_table.py b/tests/unit/models/dashboard/test_dashboard_table.py
@@ -31,3 +31,30 @@ def test_dashboard_table_relations(self) -> None:
                     RELATION_REVERSE_TYPE: 'TABLE_OF_DASHBOARD'}
         assert actual is not None
         self.assertDictEqual(actual, expected)
+
+    def test_dashboard_table_without_dot_as_name(self) -> None:
+        dashboard_table = DashboardTable(table_ids=['bq-name://project-id.schema-name/table-name'],
+                                         cluster='cluster_id', product='product_id',
+                                         dashboard_id='dashboard_id', dashboard_group_id='dashboard_group_id')
+        actual = dashboard_table.create_next_relation()
+        expected = {RELATION_END_KEY: 'bq-name://project-id.schema-name/table-name', RELATION_START_LABEL: 'Dashboard',
+                    RELATION_END_LABEL: 'Table',
+                    RELATION_START_KEY: 'product_id_dashboard://cluster_id.dashboard_group_id/dashboard_id',
+                    RELATION_TYPE: 'DASHBOARD_WITH_TABLE',
+                    RELATION_REVERSE_TYPE: 'TABLE_OF_DASHBOARD'}
+        assert actual is not None
+        self.assertDictEqual(actual, expected)
+
+    def test_dashboard_table_with_dot_as_name(self) -> None:
+        dashboard_table = DashboardTable(table_ids=['bq-name://project.id.schema-name/table-name'],
+                                         cluster='cluster_id', product='product_id',
+                                         dashboard_id='dashboard_id', dashboard_group_id='dashboard_group_id')
+        actual = dashboard_table.create_next_relation()
+        self.assertIsNone(actual)
+
+    def test_dashboard_table_with_slash_as_name(self) -> None:
+        dashboard_table = DashboardTable(table_ids=['bq/name://project/id.schema/name/table/name'],
+                                         cluster='cluster_id', product='product_id',
+                                         dashboard_id='dashboard_id', dashboard_group_id='dashboard_group_id')
+        actual = dashboard_table.create_next_relation()
+        self.assertIsNone(actual)
diff --git a/tests/unit/transformer/test_timestamp_string_to_epoch_transformer.py b/tests/unit/transformer/test_timestamp_string_to_epoch_transformer.py
@@ -33,6 +33,15 @@ def test_conversion_with_format(self) -> None:
         actual = transformer.transform({'foo': '2020-02-19T19:52:33Z'})
         self.assertDictEqual({'foo': 1582141953}, actual)
 
+    def test_invalid_timestamp(self) -> None:
+        transformer = TimestampStringToEpoch()
+        config = ConfigFactory.from_dict({
+            FIELD_NAME: 'foo',
+        })
+        transformer.init(conf=config)
+        actual = transformer.transform({'foo': '165de33266d4'})
+        self.assertEquals(actual['foo'], 0)
+
 
 if __name__ == '__main__':
     unittest.main()