Source Google Ads: refactor error messages; add pattern_descriptor (#…

…26948) * Source Google Ads: refactor error messages; add spec patters * Source Google Ads: update docs
airbytehq · Jun 2, 2023 · 32a4ba1 · 32a4ba1
1 parent 0f887bc
commit 32a4ba1
Show file tree

Hide file tree

Showing 11 changed files with 60 additions and 35 deletions.
diff --git a/airbyte-integrations/connectors/source-google-ads/Dockerfile b/airbyte-integrations/connectors/source-google-ads/Dockerfile
@@ -13,5 +13,5 @@ COPY main.py ./
 
 ENTRYPOINT ["python", "/airbyte/integration_code/main.py"]
 
-LABEL io.airbyte.version=0.2.21
+LABEL io.airbyte.version=0.2.22
 LABEL io.airbyte.name=airbyte/source-google-ads
diff --git a/airbyte-integrations/connectors/source-google-ads/metadata.yaml b/airbyte-integrations/connectors/source-google-ads/metadata.yaml
@@ -6,7 +6,7 @@ data:
   connectorSubtype: api
   connectorType: source
   definitionId: 253487c0-2246-43ba-a21f-5116b20a2c50
-  dockerImageTag: 0.2.21
+  dockerImageTag: 0.2.22
   dockerRepository: airbyte/source-google-ads
   githubIssueLabel: source-google-ads
   icon: google-adwords.svg

diff --git a/airbyte-integrations/connectors/source-google-ads/source_google_ads/google_ads.py b/airbyte-integrations/connectors/source-google-ads/source_google_ads/google_ads.py
@@ -55,7 +55,7 @@ def get_google_ads_client(credentials) -> GoogleAdsClient:
         try:
             return GoogleAdsClient.load_from_dict(credentials, version=API_VERSION)
         except exceptions.RefreshError as e:
-            message = f"Config Error, Please Check permissions: {str(e)}"
+            message = "The authentication to Google Ads has expired. Re-authenticate to restore access to Google Ads."
             raise AirbyteTracedException(message=message, failure_type=FailureType.config_error) from e
 
     @backoff.on_exception(

diff --git a/airbyte-integrations/connectors/source-google-ads/source_google_ads/source.py b/airbyte-integrations/connectors/source-google-ads/source_google_ads/source.py
@@ -7,9 +7,10 @@
 import traceback
 from typing import Any, Iterable, List, Mapping, MutableMapping, Tuple
 
-from airbyte_cdk.models import SyncMode
+from airbyte_cdk.models import FailureType, SyncMode
 from airbyte_cdk.sources import AbstractSource
 from airbyte_cdk.sources.streams import Stream
+from airbyte_cdk.utils import AirbyteTracedException
 from google.ads.googleads.errors import GoogleAdsException
 from pendulum import parse, today
 
@@ -46,7 +47,11 @@ def _validate_and_transform(config: Mapping[str, Any]):
         if config.get("end_date") == "":
             config.pop("end_date")
         for query in config.get("custom_queries", []):
-            query["query"] = GAQL.parse(query["query"])
+            try:
+                query["query"] = GAQL.parse(query["query"])
+            except ValueError:
+                message = f"The custom GAQL query {query['table_name']} failed. Validate your GAQL query with the Google Ads query validator. https://developers.google.com/google-ads/api/fields/v13/query_validator"
+                raise AirbyteTracedException(message=message, failure_type=FailureType.config_error)
         return config
 
     @staticmethod

diff --git a/airbyte-integrations/connectors/source-google-ads/source_google_ads/spec.json b/airbyte-integrations/connectors/source-google-ads/source_google_ads/spec.json
@@ -60,6 +60,7 @@
         "type": "string",
         "description": "Comma separated list of (client) customer IDs. Each customer ID must be specified as a 10-digit number without dashes. More instruction on how to find this value in our <a href=\"https://docs.airbyte.com/integrations/sources/google-ads#setup-guide\">docs</a>. Metrics streams like AdGroupAdReport cannot be requested for a manager account.",
         "pattern": "^[0-9]{10}(,[0-9]{10})*$",
+        "pattern_descriptor": "^[0-9]{10}(,[0-9]{10})*$ . The customer ID must be 10 digits. Separate multiple customer IDs using commas.",
         "examples": ["6783948572,5839201945"],
         "order": 1
       },
@@ -68,6 +69,7 @@
         "title": "Start Date",
         "description": "UTC date and time in the format 2017-01-25. Any data before this date will not be replicated.",
         "pattern": "^[0-9]{4}-[0-9]{2}-[0-9]{2}$",
+        "pattern_descriptor": "YYYY-MM-DD",
         "examples": ["2017-01-25"],
         "order": 2,
         "format": "date"
@@ -77,6 +79,7 @@
         "title": "End Date",
         "description": "UTC date and time in the format 2017-01-25. Any data after this date will not be replicated.",
         "pattern": "^$|^[0-9]{4}-[0-9]{2}-[0-9]{2}$",
+        "pattern_descriptor": "YYYY-MM-DD",
         "examples": ["2017-01-30"],
         "order": 6,
         "format": "date"

diff --git a/airbyte-integrations/connectors/source-google-ads/source_google_ads/streams.py b/airbyte-integrations/connectors/source-google-ads/source_google_ads/streams.py
@@ -131,8 +131,8 @@ def read_records(self, sync_mode, stream_slice: Optional[Mapping[str, Any]] = No
             return []
 
         customer_id = stream_slice["customer_id"]
-        response_records = self.google_ads_client.send_request(self.get_query(stream_slice), customer_id=customer_id)
         try:
+            response_records = self.google_ads_client.send_request(self.get_query(stream_slice), customer_id=customer_id)
             for response in response_records:
                 yield from self.parse_response(response)
         except GoogleAdsException as exc:

diff --git a/airbyte-integrations/connectors/source-google-ads/source_google_ads/utils.py b/airbyte-integrations/connectors/source-google-ads/source_google_ads/utils.py
@@ -6,9 +6,6 @@
 from dataclasses import dataclass
 from typing import Optional, Tuple
 
-from airbyte_cdk.models import FailureType
-from airbyte_cdk.utils import AirbyteTracedException
-
 
 @dataclass(repr=False, eq=False, frozen=True)
 class GAQL:
@@ -44,23 +41,17 @@ class GAQL:
     @classmethod
     def parse(cls, query):
         m = cls.REGEX.match(query)
-
-        internal_message = f"Incorrect GAQL query statement: {repr(query)}"
-        message = f"The GAQL query statement is incorrect: {repr(query)}"
-        query_error = AirbyteTracedException(message=message, internal_message=internal_message, failure_type=FailureType.config_error)
-
         if not m:
-            raise query_error
+            raise ValueError
 
         fields = [f.strip() for f in m.group("FieldNames").split(",")]
         for field in fields:
             if not cls.REGEX_FIELD_NAME.match(field):
-                raise query_error
+                raise ValueError
 
         resource_names = re.split(r"\s*,\s*", m.group("ResourceNames"))
         if len(resource_names) > 1:
-            message = f"Incorrect GAQL query: multiple resources '{', '.join(resource_names)}' is not allowed"
-            raise AirbyteTracedException(message=message, internal_message=message, failure_type=FailureType.config_error)
+            raise ValueError
         resource_name = resource_names[0]
 
         where = cls._normalize(m.group("WhereClause") or "")

diff --git a/airbyte-integrations/connectors/source-google-ads/unit_tests/test_google_ads.py b/airbyte-integrations/connectors/source-google-ads/unit_tests/test_google_ads.py
@@ -64,7 +64,8 @@ def test_google_ads_wrong_permissions(mocker):
     mocker.patch("source_google_ads.google_ads.GoogleAdsClient.load_from_dict", side_effect=exceptions.RefreshError("invalid_grant"))
     with pytest.raises(AirbyteTracedException) as e:
         GoogleAds(**SAMPLE_CONFIG)
-    assert e.value.message == "Config Error, Please Check permissions: invalid_grant"
+    expected_message = "The authentication to Google Ads has expired. Re-authenticate to restore access to Google Ads."
+    assert e.value.message == expected_message
 
 
 def test_send_request(mocker, customers):

diff --git a/airbyte-integrations/connectors/source-google-ads/unit_tests/test_source.py b/airbyte-integrations/connectors/source-google-ads/unit_tests/test_source.py
@@ -503,7 +503,7 @@ def test_invalid_custom_query_handled(mocked_gads_api, config):
         (ServiceAccounts, "internal_error", 1, True),
     ),
 )
-def test_read_record_error_handling(config, customers, caplog, mocked_gads_api, cls, error, failure_code, raise_expected):
+def test_read_record_error_handling(config, customers, mocked_gads_api, cls, error, failure_code, raise_expected):
     error_msg = "Some unexpected error"
     mocked_gads_api(failure_code=failure_code, failure_msg=error_msg, error_type=error)
     google_api = GoogleAds(credentials=config["credentials"])

diff --git a/airbyte-integrations/connectors/source-google-ads/unit_tests/test_utils.py b/airbyte-integrations/connectors/source-google-ads/unit_tests/test_utils.py
@@ -4,6 +4,7 @@
 
 import pytest
 from airbyte_cdk.utils import AirbyteTracedException
+from source_google_ads import SourceGoogleAds
 from source_google_ads.utils import GAQL
 
 
@@ -70,21 +71,44 @@ def test_parse_GAQL_ok():
     assert str(sql) == "SELECT field1, field2 FROM x_Table WHERE date = '2020-01-01' ORDER BY field2 ASC, field1 DESC LIMIT 10 PARAMETERS include_drafts=true"
 
 
-def test_parse_GAQL_fail():
-    with pytest.raises(AirbyteTracedException) as e:
-        GAQL.parse("SELECT field1, field2 FROM x_Table2")
-    assert str(e.value) == "Incorrect GAQL query statement: 'SELECT field1, field2 FROM x_Table2'"
-
-    with pytest.raises(AirbyteTracedException) as e:
-        GAQL.parse("SELECT field1, field2 FROM x_Table WHERE ")
-    with pytest.raises(AirbyteTracedException) as e:
-        GAQL.parse("SELECT field1, , field2 FROM table")
-    with pytest.raises(AirbyteTracedException) as e:
-        GAQL.parse("SELECT fie ld1, field2 FROM table")
-
+@pytest.mark.parametrize(
+    "config",
+    [
+        {
+            "custom_queries": [
+                {
+                    "query": "SELECT field1, field2 FROM x_Table2",
+                    "table_name": "test_table"
+                }]
+        },
+        {
+            "custom_queries": [
+                {
+                    "query": "SELECT field1, field2 FROM x_Table WHERE ",
+                    "table_name": "test_table"
+                }]
+        },
+        {
+            "custom_queries": [
+                {
+                    "query": "SELECT field1, , field2 FROM table",
+                    "table_name": "test_table"
+                }]
+        },
+        {
+            "custom_queries": [
+                {
+                    "query": "SELECT fie ld1, field2 FROM table",
+                    "table_name": "test_table"
+                }]
+        },
+    ]
+)
+def test_parse_GAQL_fail(config):
     with pytest.raises(AirbyteTracedException) as e:
-        GAQL.parse("SELECT field1, field2 FROM customer, campaign_labels")
-    assert str(e.value) == "Incorrect GAQL query: multiple resources 'customer, campaign_labels' is not allowed"
+        SourceGoogleAds._validate_and_transform(config)
+    expected_message = "The custom GAQL query test_table failed. Validate your GAQL query with the Google Ads query validator. https://developers.google.com/google-ads/api/fields/v13/query_validator"
+    assert e.value.message == expected_message
 
 
 @pytest.mark.parametrize(

diff --git a/docs/integrations/sources/google-ads.md b/docs/integrations/sources/google-ads.md
@@ -152,7 +152,8 @@ Due to a limitation in the Google Ads API which does not allow getting performan
 
 | Version  | Date       | Pull Request                                             | Subject                                                                                                                              |
 |:---------|:-----------|:---------------------------------------------------------|:-------------------------------------------------------------------------------------------------------------------------------------|
-| `0.2.21` | 2023-05-30 | [25314](https://github.com/airbytehq/airbyte/pull/25314) | add full refresh custom table `asset_group_listing_group_filter`                                                                    |
+| `0.2.22` | 2023-06-02 | [26948](https://github.com/airbytehq/airbyte/pull/26948) | Refactor error messages; add `pattern_descriptor` for fields in spec                                                                 |
+| `0.2.21` | 2023-05-30 | [25314](https://github.com/airbytehq/airbyte/pull/25314) | add full refresh custom table `asset_group_listing_group_filter`                                                                     |
 | `0.2.20` | 2023-05-30 | [25624](https://github.com/airbytehq/airbyte/pull/25624) | Add `asset` Resource to full refresh custom tables (GAQL Queries)                                                                    |
 | `0.2.19` | 2023-05-15 | [26209](https://github.com/airbytehq/airbyte/pull/26209) | Handle Token Refresh errors as `config_error`                                                                                        |
 | `0.2.18` | 2023-05-15 | [25947](https://github.com/airbytehq/airbyte/pull/25947) | Improve GAQL parser error message if multiple resources provided                                                                     |