Skip to content

Google Ads: Fix wrong type for metrics.conversations #19030

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

Moandco
Copy link

@Moandco Moandco commented Nov 7, 2022

What

Fixes Google ads sync failing during normalization dbt run when Google ads API returns non-integer value for metrics.conversations. Currently the field is expected to be integer, however Google might return floats.
Please refer to the documentation of the field: https://developers.google.com/google-ads/api/fields/v11/metrics#metrics.conversions

How

Change type from integer to number in schema json.

🚨 User Impact 🚨

No breaking changes

Pre-merge Checklist

Expand the relevant checklist and delete the others.

Updating a connector

Community member or Airbyter

  • Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

============================= test session starts =============================
collecting ... collected 47 items

integration_tests\test_incremental.py::test_incremental_sync 
integration_tests\test_incremental.py::test_abnormally_large_state 
unit_tests\test_custom_query.py::test_custom_query 
unit_tests\test_google_ads.py::test_google_ads_init 
unit_tests\test_models.py::test_manager_account[True] 
unit_tests\test_models.py::test_manager_account[False] 
unit_tests\test_source.py::test_chunk_date_range_without_end_date 
unit_tests\test_source.py::test_chunk_date_range 
unit_tests\test_source.py::test_streams_count 
unit_tests\test_source.py::test_metrics_in_custom_query[SELECT customer.id, metrics.conversions, campaign.start_date FROM campaign-True] 
unit_tests\test_source.py::test_metrics_in_custom_query[SELECT segments.ad_destination_type, campaign.start_date, campaign.end_date FROM campaign-False] 
unit_tests\test_source.py::test_updated_state[latest_record0-current_state0-expected_state0] 
unit_tests\test_source.py::test_updated_state[latest_record1-current_state1-expected_state1] 
unit_tests\test_source.py::test_updated_state[latest_record2-current_state2-expected_state2] 
unit_tests\test_source.py::test_get_query_fields[\nSELECT\n  campaign.id,\n  campaign.name,\n  campaign.status,\n  metrics.impressions\nFROM campaign\nWHERE campaign.status = 'PAUSED'\nAND metrics.impressions > 100\nORDER BY campaign.status\n    -fields0] 
unit_tests\test_source.py::test_get_query_fields[\nSELECT\n  campaign.accessible_bidding_strategy,\n  segments.ad_destination_type,\n  campaign.start_date,\n  campaign.end_date\nFROM campaign\n    -fields1] 
unit_tests\test_source.py::test_get_query_fields[selet aasdasd from aaa-fields2] 
unit_tests\test_source.py::test_insert_date[\nSELECT\n  campaign.id,\n  campaign.name,\n  campaign.status,\n  metrics.impressions\nFROM campaign\nWHERE campaign.status = 'PAUSED'\nAND metrics.impressions > 100\nORDER BY campaign.status\n-\nSELECT\n  campaign.id,\n  campaign.name,\n  campaign.status,\n  metrics.impressions\n, segments.date\nFROM campaign\nWHERE campaign.status = 'PAUSED'\nAND metrics.impressions > 100\n AND segments.date BETWEEN '1980-01-01' AND '2000-01-01'\nORDER BY campaign.status\n] 
unit_tests\test_source.py::test_insert_date[\nSELECT\n  campaign.id,\n  campaign.name,\n  campaign.status,\n  metrics.impressions\nFROM campaign\nORDER BY campaign.status\n-\nSELECT\n  campaign.id,\n  campaign.name,\n  campaign.status,\n  metrics.impressions\n, segments.date\nFROM campaign\n\nWHERE segments.date BETWEEN '1980-01-01' AND '2000-01-01'\nORDER BY campaign.status\n] 
unit_tests\test_source.py::test_insert_date[\nSELECT\n  campaign.id,\n  campaign.name,\n  campaign.status,\n  metrics.impressions\nFROM campaign\nWHERE campaign.status = 'PAUSED'\nAND metrics.impressions > 100\n-\nSELECT\n  campaign.id,\n  campaign.name,\n  campaign.status,\n  metrics.impressions\n, segments.date\nFROM campaign\nWHERE campaign.status = 'PAUSED'\nAND metrics.impressions > 100\n AND segments.date BETWEEN '1980-01-01' AND '2000-01-01'\n] 
unit_tests\test_source.py::test_insert_date[\nSELECT\n    campaign.accessible_bidding_strategy,\n    segments.ad_destination_type,\n    campaign.start_date,\n    campaign.end_date\nFROM campaign\n-\nSELECT\n    campaign.accessible_bidding_strategy,\n    segments.ad_destination_type,\n    campaign.start_date,\n    campaign.end_date\n, segments.date\nFROM campaign\n\nWHERE segments.date BETWEEN '1980-01-01' AND '2000-01-01'\n] 
unit_tests\test_source.py::test_get_json_schema_parse_query 
unit_tests\test_source.py::test_get_json_schema_parse_query_with_end_date 
unit_tests\test_source.py::test_google_type_conversion 
unit_tests\test_source.py::test_check_connection_should_pass_when_config_valid 
unit_tests\test_source.py::test_check_connection_should_fail_when_api_call_fails 
unit_tests\test_source.py::test_end_date_is_not_in_the_future 
unit_tests\test_source.py::test_invalid_custom_query_handled 
unit_tests\test_source.py::test_read_record_error_handling[AdGroupLabels-authorization_error-AuthorizationError.CUSTOMER_NOT_ENABLED-False-True] 
unit_tests\test_streams.py::test_page_token_expired_retry_succeeds 
unit_tests\test_streams.py::test_page_token_expired_retry_fails 
unit_tests\test_streams.py::test_page_token_expired_it_should_fail_date_range_1_day 

====================== 47 passed, 292 warnings in 49.80s ======================

Process finished with exit code 0
{"type": "LOG", "log": {"level": "INFO", "message": "Starting syncing SourceGoogleAds"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Request made: ClientCustomerId: REMOVED, Host: googleads.googleapis.com, Method: /google.ads.googleads.v11.services.GoogleAdsService/Search, RequestId: t-m7QCCOpwi3Xk-2Tm5Dcw, IsFault: False, FaultMessage: None"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Syncing stream: ad_group_ad_report "}}
{"type": "LOG", "log": {"level": "INFO", "message": "Request made: ClientCustomerId: REMOVED, Host: googleads.googleapis.com, Method: /google.ads.googleads.v11.services.GoogleAdsService/Search, RequestId: WAFalWXvqh3NQ4BBpaH_vQ, IsFault: False, FaultMessage: None"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Request made: ClientCustomerId: REMOVED, Host: googleads.googleapis.com, Method: /google.ads.googleads.v11.services.GoogleAdsService/Search, RequestId: 6D7jMKiQUu2PWKpNAamvuA, IsFault: False, FaultMessage: None"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Request made: ClientCustomerId: REMOVED, Host: googleads.googleapis.com, Method: /google.ads.googleads.v11.services.GoogleAdsService/Search, RequestId: oTe_UKlfHx467dBc-fzbUg, IsFault: False, FaultMessage: None"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Request made: ClientCustomerId: REMOVED, Host: googleads.googleapis.com, Method: /google.ads.googleads.v11.services.GoogleAdsService/Search, RequestId: s0Z0aITipbq31YqMQ25TaA, IsFault: False, FaultMessage: None"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Request made: ClientCustomerId: REMOVED, Host: googleads.googleapis.com, Method: /google.ads.googleads.v11.services.GoogleAdsService/Search, RequestId: _LFIzI296T51lPRFp2qKjw, IsFault: False, FaultMessage: None"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Request made: ClientCustomerId: REMOVED, Host: googleads.googleapis.com, Method: /google.ads.googleads.v11.services.GoogleAdsService/Search, RequestId: Qi6XDs3rw7MmxejAr2BAUA, IsFault: False, FaultMessage: None"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Request made: ClientCustomerId: REMOVED, Host: googleads.googleapis.com, Method: /google.ads.googleads.v11.services.GoogleAdsService/Search, RequestId: 2Az1E-mvBtQ4GIve8mYnnw, IsFault: False, FaultMessage: None"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Request made: ClientCustomerId: REMOVED, Host: googleads.googleapis.com, Method: /google.ads.googleads.v11.services.GoogleAdsService/Search, RequestId: Pu5_GF5y-7WUscyNyx_k9Q, IsFault: False, FaultMessage: None"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Read 246 records from ad_group_ad_report stream"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing ad_group_ad_report"}}
{"type": "LOG", "log": {"level": "INFO", "message": "SourceGoogleAds runtimes:\nSyncing stream ad_group_ad_report 0:00:42.380093"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing SourceGoogleAds"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Starting syncing SourceGoogleAds"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Request made: ClientCustomerId: REMOVED, Host: googleads.googleapis.com, Method: /google.ads.googleads.v11.services.GoogleAdsService/Search, RequestId: 3aDH--tX7pZqzyZk98VXhQ, IsFault: False, FaultMessage: None"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Syncing stream: ad_group_ad_report "}}
{"type": "LOG", "log": {"level": "INFO", "message": "Setting state of ad_group_ad_report stream to {'REMOVED': {'segments.date': '2022-11-08'}}"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Request made: ClientCustomerId: REMOVED, Host: googleads.googleapis.com, Method: /google.ads.googleads.v11.services.GoogleAdsService/Search, RequestId: 66Xdj_DdIogYsD3kPCga_g, IsFault: False, FaultMessage: None"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Read 42 records from ad_group_ad_report stream"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing ad_group_ad_report"}}
{"type": "LOG", "log": {"level": "INFO", "message": "SourceGoogleAds runtimes:\nSyncing stream ad_group_ad_report 0:00:02.113350"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing SourceGoogleAds"}}
PASSED{"type": "LOG", "log": {"level": "INFO", "message": "Starting syncing SourceGoogleAds"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Request made: ClientCustomerId: REMOVED, Host: googleads.googleapis.com, Method: /google.ads.googleads.v11.services.GoogleAdsService/Search, RequestId: sq_jgLw9o1dNDQrdjRK4LQ, IsFault: False, FaultMessage: None"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Syncing stream: ad_group_ad_report "}}
{"type": "LOG", "log": {"level": "INFO", "message": "Setting state of ad_group_ad_report stream to {'segments.date': '2222-06-04'}"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Read 0 records from ad_group_ad_report stream"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing ad_group_ad_report"}}
{"type": "LOG", "log": {"level": "INFO", "message": "SourceGoogleAds runtimes:\nSyncing stream ad_group_ad_report 0:00:00.000846"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing SourceGoogleAds"}}
PASSEDPASSEDPASSED
unit_tests\test_google_ads.py::test_send_request PASSED
unit_tests\test_google_ads.py::test_get_fields_from_schema PASSED
unit_tests\test_google_ads.py::test_interval_chunking PASSED
unit_tests\test_google_ads.py::test_get_date_params PASSED
unit_tests\test_google_ads.py::test_get_date_params_with_time_zone PASSED
unit_tests\test_google_ads.py::test_convert_schema_into_query PASSED
unit_tests\test_google_ads.py::test_get_field_value PASSED
unit_tests\test_google_ads.py::test_parse_single_result PASSED
unit_tests\test_google_ads.py::test_get_date_params_with_date PASSED
unit_tests\test_google_ads.py::test_get_date_params_without_end_date PASSED
unit_tests\test_models.py::test_time_zone PASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSEDPASSED{"type": "LOG", "log": {"level": "INFO", "message": "Checking the config"}}
PASSED{"type": "LOG", "log": {"level": "INFO", "message": "Checking the config"}}
{"type": "LOG", "log": {"level": "ERROR", "message": "Traceback (most recent call last):\n  File \"C:\\Users\\MoritzDinger\\repos\\airbyte\\airbyte-integrations\\connectors\\source-google-ads\\source_google_ads\\source.py\", line 94, in check_connection\n    customers = Customer.from_accounts(accounts)\n  File \"C:\\Users\\MoritzDinger\\repos\\airbyte\\airbyte-integrations\\connectors\\source-google-ads\\source_google_ads\\models.py\", line 22, in from_accounts\n    for account in account_list:\n  File \"C:\\Users\\MoritzDinger\\repos\\airbyte\\airbyte-integrations\\connectors\\source-google-ads\\source_google_ads\\streams.py\", line 111, in read_records\n    response_records = self.google_ads_client.send_request(self.get_query(stream_slice), customer_id=customer_id)\n  File \"C:\\Users\\MoritzDinger\\repos\\airbyte\\airbyte-integrations\\connectors\\source-google-ads\\unit_tests\\common.py\", line 63, in send_request\n    raise make_google_ads_exception(1)\ngoogle.ads.googleads.errors.GoogleAdsException: (None, None, errors {\n  error_code {\n    request_error: UNKNOWN\n  }\n  message: \"it failed\"\n}\nrequest_id: \"1\"\n, 1)\n"}}
PASSEDPASSED{"type": "LOG", "log": {"level": "INFO", "message": "Checking the config"}}
{"type": "LOG", "log": {"level": "ERROR", "message": "Traceback (most recent call last):\n  File \"C:\\Users\\MoritzDinger\\repos\\airbyte\\airbyte-integrations\\connectors\\source-google-ads\\source_google_ads\\source.py\", line 109, in check_connection\n    for _ in response:\n  File \"C:\\Users\\MoritzDinger\\repos\\airbyte\\airbyte-integrations\\connectors\\source-google-ads\\unit_tests\\test_source.py\", line 47, in side_effect_func\n    raise make_google_ads_exception(failure_code=failure_code, failure_msg=failure_msg, error_type=error_type)\ngoogle.ads.googleads.errors.GoogleAdsException: (None, None, errors {\n  error_code {\n    request_error: UNKNOWN\n  }\n  message: \"Unrecognized field in the query: \\'ad_group_ad.ad.video_ad.media_file\\'\"\n}\nrequest_id: \"1\"\n, 1)\n"}}
PASSED{"type": "LOG", "log": {"level": "ERROR", "message": "Some unexpected error"}}
PASSED
unit_tests\test_source.py::test_read_record_error_handling[AdGroupLabels-internal_error-1-True-False] PASSED
unit_tests\test_source.py::test_read_record_error_handling[ServiceAccounts-authentication_error-1-True-False] PASSED
unit_tests\test_source.py::test_read_record_error_handling[ServiceAccounts-internal_error-1-True-False] PASSED
unit_tests\test_source.py::test_stream_slices PASSEDPASSED{"type": "LOG", "log": {"level": "ERROR", "message": "Page token has expired."}}
PASSED{"type": "LOG", "log": {"level": "ERROR", "message": "Page token has expired."}}
Acceptance

Put your acceptance tests output here.

@CLAassistant
Copy link

CLAassistant commented Nov 7, 2022

CLA assistant check
All committers have signed the CLA.

@tuanchris tuanchris self-assigned this Nov 7, 2022
@tuanchris tuanchris self-requested a review November 7, 2022 17:08
@tuanchris
Copy link
Contributor

tuanchris commented Nov 7, 2022

Hi, @Moandco, thanks for the PR. This looks pretty straight forward. Can you:

  • Share a screenshot of integration tests passing
  • Bump the version of the connector
  • Add a changelog

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Nov 8, 2022
@Moandco
Copy link
Author

Moandco commented Nov 8, 2022

Hi @tuanchris, thanks for your review. I updated the changelog and put the unit and integration test output inside the PR description. Not sure where to bump the version of the connector.

@tuanchris
Copy link
Contributor

Hi @Moandco, can you follow step 1 & 4 under publishing a connector?

@Moandco
Copy link
Author

Moandco commented Nov 8, 2022

@tuanchris Thanks for the precise information. I did the bump in the docker file. Right now step 4 fails with that:

Failed to fetch valid spec file for docker image airbyte/source-google-ads:0.2.4 from GCS bucket io-airbyte-cloud-spec-cache. This will continue to fail until the connector change has been approved and published.

The documentation states your action is required. Thanks.

@tuanchris
Copy link
Contributor

tuanchris commented Nov 9, 2022

/test connector=connectors/source-google-ads

🕑 connectors/source-google-ads https://github.com/airbytehq/airbyte/actions/runs/3424691205
❌ connectors/source-google-ads https://github.com/airbytehq/airbyte/actions/runs/3424691205
🐛 https://gradle.com/s/lodel6jtfpqfm

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestDiscovery::test_backward_compatibility[inputs0] - so...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
======= 1 failed, 27 passed, 1 skipped, 27 warnings in 708.67s (0:11:48) =======

> Task :airbyte-integrations:connectors:source-google-ads:sourceAcceptanceTest FAILED

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/7.4/userguide/command_line_interface.html#sec:command_line_warnings

Execution optimizations have been disabled for 1 invalid unit(s) of work during this build to ensure correctness.
Please consult deprecation warnings for more details.
50 actionable tasks: 37 executed, 13 up-to-date

Publishing build scan...
https://gradle.com/s/lodel6jtfpqfm

@Moandco
Copy link
Author

Moandco commented Nov 9, 2022

@tuanchris Thanks for triggering the build. There is one error related to the type change. I don't know what to do about it. Can you help?

source_acceptance_test.utils.backward_compatibility.NonBackwardCompatibleError: BackwardIncompatibilityContext.DISCOVER - The'type' field value was changed.. Diff: Item root['campaigns']['properties']['metrics.conversions']['type'][1] removed from iterable.

@tuanchris
Copy link
Contributor

@Moandco can you bump the version here?
image

@tuanchris
Copy link
Contributor

tuanchris commented Nov 9, 2022

/test connector=connectors/source-google-ads

🕑 connectors/source-google-ads https://github.com/airbytehq/airbyte/actions/runs/3428343955
❌ connectors/source-google-ads https://github.com/airbytehq/airbyte/actions/runs/3428343955
🐛 https://gradle.com/s/xlizu4wo4da6o

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestDiscovery::test_backward_compatibility[inputs0] - so...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
======= 1 failed, 27 passed, 1 skipped, 27 warnings in 819.14s (0:13:39) =======

> Task :airbyte-integrations:connectors:source-google-ads:sourceAcceptanceTest FAILED

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/7.4/userguide/command_line_interface.html#sec:command_line_warnings

Execution optimizations have been disabled for 1 invalid unit(s) of work during this build to ensure correctness.
Please consult deprecation warnings for more details.
50 actionable tasks: 37 executed, 13 up-to-date

Publishing build scan...
https://gradle.com/s/xlizu4wo4da6o

@Moandco
Copy link
Author

Moandco commented Nov 9, 2022

Test failed with the same error again. 😞

@Moandco
Copy link
Author

Moandco commented Nov 10, 2022

The issue was fixed with this PR already. #19208

@Moandco Moandco closed this Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation bounty community connectors/source/google-ads
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

4 participants