Skip to content

Commit

Permalink
Source Google Ads: temporary patch to avoid 500 Internal server error (
Browse files Browse the repository at this point in the history
  • Loading branch information
roman-yermilov-gl authored and jatinyadav-cc committed Feb 26, 2024
1 parent 9601083 commit aab69fb
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 26 deletions.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Expand Up @@ -11,7 +11,7 @@ data:
connectorSubtype: api
connectorType: source
definitionId: 253487c0-2246-43ba-a21f-5116b20a2c50
dockerImageTag: 3.3.2
dockerImageTag: 3.3.3
dockerRepository: airbyte/source-google-ads
documentationUrl: https://docs.airbyte.com/integrations/sources/google-ads
githubIssueLabel: source-google-ads
Expand Down
Expand Up @@ -5,7 +5,7 @@

import logging
from enum import Enum
from typing import Any, Iterable, Iterator, List, Mapping, MutableMapping
from typing import Any, Iterable, Iterator, List, Mapping, MutableMapping, Optional

import backoff
from airbyte_cdk.models import FailureType
Expand Down Expand Up @@ -214,8 +214,18 @@ def get_field_value(field_value: GoogleAdsRow, field: str, schema_type: Mapping[
return field_value

@staticmethod
def parse_single_result(schema: Mapping[str, Any], result: GoogleAdsRow):
def parse_single_result(schema: Mapping[str, Any], result: GoogleAdsRow, nullable: Optional[List[str]] = None):
if nullable is None:
nullable = []
props = schema.get("properties")
fields = GoogleAds.get_fields_from_schema(schema)
single_record = {field: GoogleAds.get_field_value(result, field, props.get(field)) for field in fields}
# Making fields nullable is a temporary try to avoid `500 Internal server error` happen when
# `user_interest.availabilities` and `user_interest.launched_to_all` fields are queried while syncing `user_interest` stream.
# It seems like the values for the fields have changed following the 500 errors and it is unexpected,
# so until we understand the issue with the 500 errors, we will nullify those two fields.
# This should affect only the `user_interest` stream.
# TODO: we need to get rid of the nullable condition once the issue https://github.com/airbytehq/oncall/issues/4306 to be resolved.
single_record = {
field: GoogleAds.get_field_value(result, field, props.get(field)) if field not in nullable else None for field in fields
}
return single_record
Expand Up @@ -31,14 +31,24 @@ def __init__(self, api: GoogleAds, customers: List[CustomerModel]):
self.customers = customers

def get_query(self, stream_slice: Mapping[str, Any]) -> str:
fields = GoogleAds.get_fields_from_schema(self.get_json_schema())
# The following construction excludes "user_interest.availabilities" and "user_interest.launched_to_all" fields
# from the query because they are a cause of "500 Internal server error" on the google side.
# The oncall issue: https://github.com/airbytehq/oncall/issues/4306
# TODO: replace with fields = GoogleAds.get_fields_from_schema(self.get_json_schema())
fields = [
field
for field in GoogleAds.get_fields_from_schema(self.get_json_schema())
if not (field == "user_interest.availabilities" or field == "user_interest.launched_to_all")
]
table_name = get_resource_name(self.name)
query = GoogleAds.convert_schema_into_query(fields=fields, table_name=table_name)
return query

def parse_response(self, response: SearchPager, stream_slice: Optional[Mapping[str, Any]] = None) -> Iterable[Mapping]:
for result in response:
yield self.google_ads_client.parse_single_result(self.get_json_schema(), result)
yield self.google_ads_client.parse_single_result(
self.get_json_schema(), result, nullable=["user_interest.availabilities", "user_interest.launched_to_all"]
)

def stream_slices(self, stream_state: Mapping[str, Any] = None, **kwargs) -> Iterable[Optional[Mapping[str, any]]]:
for customer in self.customers:
Expand Down
Expand Up @@ -51,7 +51,7 @@ def mock_response_child():


class MockGoogleAds(GoogleAds):
def parse_single_result(self, schema, result):
def parse_single_result(self, schema, result, nullable = None):
return result

def send_request(self, query: str, customer_id: str, login_customer_id: str = "default"):
Expand Down Expand Up @@ -219,7 +219,7 @@ def mock_response_4():
class MockGoogleAdsLimit(GoogleAds):
count = 0

def parse_single_result(self, schema, result):
def parse_single_result(self, schema, result, nullable = None):
return result

def send_request(self, query: str, customer_id: str, login_customer_id: str = "default"):
Expand Down
Expand Up @@ -507,7 +507,7 @@ def test_get_customers(mocker, customer_status_filter, expected_ids, send_reques

mock_google_api.get_accessible_accounts.return_value = ["123", "789"]
mock_google_api.send_request.side_effect = mock_send_request
mock_google_api.parse_single_result.side_effect = lambda schema, result: result
mock_google_api.parse_single_result.side_effect = lambda schema, result, nullable: result

mock_config = {"customer_status_filter": customer_status_filter, "customer_ids": ["123", "456", "789"]}

Expand Down
Expand Up @@ -48,7 +48,7 @@ def mock_response_2():
class MockGoogleAds(GoogleAds):
count = 0

def parse_single_result(self, schema, result):
def parse_single_result(self, schema, result, nullable = None):
return result

def send_request(self, query: str, customer_id: str, login_customer_id: str = "none"):
Expand Down
5 changes: 3 additions & 2 deletions docs/integrations/sources/google-ads.md
Expand Up @@ -280,8 +280,9 @@ Due to a limitation in the Google Ads API which does not allow getting performan

| Version | Date | Pull Request | Subject |
|:---------|:-----------|:---------------------------------------------------------|:-------------------------------------------------------------------------------------------------------------------------------------|
| 3.3.2 | 2024-02-12 | [35158](https://github.com/airbytehq/airbyte/pull/35158) | Manage dependencies with Poetry. |
| `3.3.1` | 2024-01-16 | [34007](https://github.com/airbytehq/airbyte/pull/34007) | prepare for airbyte-lib |
| `3.3.3` | 2024-02-14 | [35280](https://github.com/airbytehq/airbyte/pull/35280) | Temporary patch that disables some fields to avoid 500 error when syncing `user_interest` steam |
| `3.3.2` | 2024-02-12 | [35158](https://github.com/airbytehq/airbyte/pull/35158) | Manage dependencies with Poetry. |
| `3.3.1` | 2024-01-16 | [34007](https://github.com/airbytehq/airbyte/pull/34007) | prepare for airbyte-lib |
| `3.3.0` | 2024-01-12 | [34212](https://github.com/airbytehq/airbyte/pull/34212) | Remove metric from query in Ad Group stream for non-manager account |
| `3.2.1` | 2024-01-12 | [34200](https://github.com/airbytehq/airbyte/pull/34200) | Disable raising error for not enabled accounts |
| `3.2.0` | 2024-01-09 | [33707](https://github.com/airbytehq/airbyte/pull/33707) | Add possibility to sync all connected accounts |
Expand Down

0 comments on commit aab69fb

Please sign in to comment.