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 Feb 14, 2024
1 parent a99bb8d commit d520990
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.

Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 d520990

Please sign in to comment.