🎉 Source Google Ads: Implement multiple customer ids for google ads #10150

yevhenii-ldv · 2022-02-07T15:54:32Z

What

resolves #6437.

How

Updated the customer_id field in the specification file so that it is possible to specify multiple Customer IDs separated by commas.

Implemented data reading for several Customer IDs using the stream_slices method which is always called before the read_records method. And also through the class variable _customer_id, in order to be able to access the current customer_id from any class method, in particular, to work with the it in get_updated_state method. Made in such a way as not to override the _read_incremental method for the SourceGoogleAds class, but to be able to work with the customer_id inside the get_updated_state method.

Pre-merge Checklist

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
Credentials added to Github CI. Instructions.
/test connector=connectors/<name> command is passing.
New Connector version released on Dockerhub by running the /publish command described here
After the new connector version is published, connector version bumped in the seed directory as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

…urochkin/implement-multiple-customer-ids-for-google-ads � Conflicts: � airbyte-integrations/connectors/source-google-ads/source_google_ads/streams.py

yevhenii-ldv · 2022-02-07T15:59:24Z

/test connector=connectors/source-google-ads

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

codecov · 2022-02-07T16:05:46Z

Codecov Report

❗ No coverage uploaded for pull request base (master@b447bb5). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head db568ea differs from pull request most recent head 7868b85. Consider uploading reports for the commit 7868b85 to get more accurate results

@@            Coverage Diff            @@
##             master   #10150   +/-   ##
=========================================
  Coverage          ?   70.05%           
=========================================
  Files             ?        5           
  Lines             ?      344           
  Branches          ?        0           
=========================================
  Hits              ?      241           
  Misses            ?      103           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b447bb5...7868b85. Read the comment docs.

yevhenii-ldv · 2022-02-07T16:28:52Z

/test connector=connectors/source-google-ads

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

yevhenii-ldv · 2022-02-07T18:45:34Z

/test connector=connectors/source-google-ads

🕑 connectors/source-google-ads https://github.com/airbytehq/airbyte/actions/runs/1808239961
❌ connectors/source-google-ads https://github.com/airbytehq/airbyte/actions/runs/1808239961
🐛 https://gradle.com/s/fndaeskfzgapk
Python short test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestConnection::test_check[inputs0] - AssertionError: as...
SKIPPED [1] ../usr/local/lib/python3.7/site-packages/source_acceptance_test/plugin.py:56: Skipping TestIncremental.test_two_sequential_reads because not found in the config
============= 1 failed, 20 passed, 1 skipped in 1225.58s (0:20:25) =============

yevhenii-ldv · 2022-02-07T20:08:56Z

/test connector=connectors/source-google-ads

🕑 connectors/source-google-ads https://github.com/airbytehq/airbyte/actions/runs/1808605450
✅ connectors/source-google-ads https://github.com/airbytehq/airbyte/actions/runs/1808605450
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/tests/test_core.py              275    106    61%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/tests/test_incremental.py        69     38    45%
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/utils/common.py                  70     17    76%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
------------------------------------------------------------------------
TOTAL                                                  876    259    70%
Name                                       Stmts   Miss  Cover
--------------------------------------------------------------
source_google_ads/__init__.py                  2      0   100%
source_google_ads/custom_query_stream.py      75     50    33%
source_google_ads/google_ads.py               70     10    86%
source_google_ads/source.py                   66     23    65%
source_google_ads/streams.py                 126     21    83%
--------------------------------------------------------------
TOTAL                                        339    104    69%
Name                                       Stmts   Miss  Cover
--------------------------------------------------------------
source_google_ads/__init__.py                  2      0   100%
source_google_ads/custom_query_stream.py      75      6    92%
source_google_ads/google_ads.py               70      6    91%
source_google_ads/source.py                   66     17    74%
source_google_ads/streams.py                 126     17    87%
--------------------------------------------------------------
TOTAL                                        339     46    86%

Python short test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.7/site-packages/source_acceptance_test/plugin.py:56: Skipping TestIncremental.test_two_sequential_reads because not found in the config
================== 21 passed, 1 skipped in 1107.69s (0:18:27) ==================

keu · 2022-02-08T22:11:59Z

airbyte-integrations/connectors/source-google-ads/source_google_ads/streams.py

-        response = self.google_ads_client.send_request(self.get_query(stream_slice))
-        yield from self.parse_response(response)
+        stream_slice = stream_slice or {}
+        account_responses = self.google_ads_client.send_request(self.get_query(stream_slice), customer_id=self._customer_id)


according to your logic customer_id will be None is this acceptable by send_request?

I updated the code a bit, overridden the stream_slices method for the class responsible for Full Refresh. Unfortunately, the solution is not very beautiful, but it allows you to avoid redefining the _read_incremental method for the SourceGoogleAds class, and at the same time be able to get the current customer_id from any class method (in particular, for the get_updated_state method).

docs/integrations/sources/google-ads.md

keu

see my comments

sergei-solonitcyn

Please, update the PR description properly.

yevhenii-ldv · 2022-02-10T23:14:44Z

/test connector=connectors/source-google-ads

🕑 connectors/source-google-ads https://github.com/airbytehq/airbyte/actions/runs/1826650434
✅ connectors/source-google-ads https://github.com/airbytehq/airbyte/actions/runs/1826650434
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/tests/test_core.py              275    106    61%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/tests/test_incremental.py        69     38    45%
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/utils/common.py                  70     17    76%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
------------------------------------------------------------------------
TOTAL                                                  876    259    70%
Name                                       Stmts   Miss  Cover
--------------------------------------------------------------
source_google_ads/__init__.py                  2      0   100%
source_google_ads/custom_query_stream.py      75     50    33%
source_google_ads/google_ads.py               68     10    85%
source_google_ads/source.py                   70     24    66%
source_google_ads/streams.py                 129     21    84%
--------------------------------------------------------------
TOTAL                                        344    105    69%
Name                                       Stmts   Miss  Cover
--------------------------------------------------------------
source_google_ads/__init__.py                  2      0   100%
source_google_ads/custom_query_stream.py      75      6    92%
source_google_ads/google_ads.py               68      6    91%
source_google_ads/source.py                   70     18    74%
source_google_ads/streams.py                 129     17    87%
--------------------------------------------------------------
TOTAL                                        344     47    86%

Python short test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.7/site-packages/source_acceptance_test/plugin.py:56: Skipping TestIncremental.test_two_sequential_reads because not found in the config
================== 21 passed, 1 skipped in 1283.62s (0:21:23) ==================

yevhenii-ldv · 2022-02-11T15:08:51Z

/publish connector=connectors/source-google-ads

🕑 connectors/source-google-ads https://github.com/airbytehq/airbyte/actions/runs/1830002185
✅ connectors/source-google-ads https://github.com/airbytehq/airbyte/actions/runs/1830002185

yevhenii-ldv added 2 commits February 4, 2022 17:39

Source Google Ads: Implement multiple Customer ID(s)

e74cb02

Source Google Ads: update states

6677d98

github-actions bot added the area/connectors Connector related issues label Feb 7, 2022

yevhenii-ldv temporarily deployed to more-secrets February 7, 2022 15:54 Inactive

update naming

0b43708

yevhenii-ldv temporarily deployed to more-secrets February 7, 2022 15:58 Inactive

Merge branch 'master' of https://github.com/airbytehq/airbyte into yk…

ec068b5

…urochkin/implement-multiple-customer-ids-for-google-ads � Conflicts: � airbyte-integrations/connectors/source-google-ads/source_google_ads/streams.py

yevhenii-ldv temporarily deployed to more-secrets February 7, 2022 16:01 Inactive

octavia-squidington-iii temporarily deployed to more-secrets February 7, 2022 16:01 Inactive

fix test

38b6f26

yevhenii-ldv temporarily deployed to more-secrets February 7, 2022 16:30 Inactive

octavia-squidington-iii temporarily deployed to more-secrets February 7, 2022 16:31 Inactive

yevhenii-ldv added 2 commits February 7, 2022 20:27

update tests

6ddbc1a

update tests

10a524c

yevhenii-ldv temporarily deployed to more-secrets February 7, 2022 18:46 Inactive

octavia-squidington-iii temporarily deployed to more-secrets February 7, 2022 18:47 Inactive

fix check_connection

d26f5d6

yevhenii-ldv temporarily deployed to more-secrets February 7, 2022 20:10 Inactive

octavia-squidington-iii temporarily deployed to more-secrets February 7, 2022 20:11 Inactive

yevhenii-ldv requested review from augan-rymkhan, htrueman, sergei-solonitcyn and Zirochkaa February 7, 2022 20:37

update version, add changelogs

0389265

github-actions bot added the area/documentation Improvements or additions to documentation label Feb 8, 2022

yevhenii-ldv temporarily deployed to more-secrets February 8, 2022 12:25 Inactive

keu reviewed Feb 8, 2022

View reviewed changes

docs/integrations/sources/google-ads.md Outdated Show resolved Hide resolved

keu suggested changes Feb 8, 2022

View reviewed changes

sergei-solonitcyn suggested changes Feb 9, 2022

View reviewed changes

update after review

db568ea

yevhenii-ldv temporarily deployed to more-secrets February 10, 2022 23:16 Inactive

octavia-squidington-iii temporarily deployed to more-secrets February 10, 2022 23:17 Inactive

yevhenii-ldv requested review from keu and sergei-solonitcyn February 11, 2022 00:01

htrueman approved these changes Feb 11, 2022

View reviewed changes

octavia-squidington-iii temporarily deployed to more-secrets February 11, 2022 15:11 Inactive

update version

7868b85

yevhenii-ldv merged commit 7e4ec32 into master Feb 11, 2022

yevhenii-ldv deleted the ykurochkin/implement-multiple-customer-ids-for-google-ads branch February 11, 2022 15:42

yevhenii-ldv temporarily deployed to more-secrets February 11, 2022 15:43 Inactive

octavia-squidington-iii mentioned this pull request Feb 12, 2022

Bump Airbyte version from 0.35.27-alpha to 0.35.28-alpha #10292

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉 Source Google Ads: Implement multiple customer ids for google ads #10150

🎉 Source Google Ads: Implement multiple customer ids for google ads #10150

yevhenii-ldv commented Feb 7, 2022 •

edited

Loading

yevhenii-ldv commented Feb 7, 2022 •

edited by github-actions bot

Loading

codecov bot commented Feb 7, 2022 •

edited

Loading

yevhenii-ldv commented Feb 7, 2022 •

edited by github-actions bot

Loading

yevhenii-ldv commented Feb 7, 2022 •

edited by github-actions bot

Loading

yevhenii-ldv commented Feb 7, 2022 •

edited by github-actions bot

Loading

keu Feb 8, 2022 •

edited

Loading

yevhenii-ldv Feb 11, 2022

keu left a comment

sergei-solonitcyn left a comment

yevhenii-ldv commented Feb 10, 2022 •

edited by github-actions bot

Loading

yevhenii-ldv commented Feb 11, 2022 •

edited by github-actions bot

Loading

🎉 Source Google Ads: Implement multiple customer ids for google ads #10150

🎉 Source Google Ads: Implement multiple customer ids for google ads #10150

Conversation

yevhenii-ldv commented Feb 7, 2022 • edited Loading

What

How

Recommended reading order

Pre-merge Checklist

Community member or Airbyter

Airbyter

yevhenii-ldv commented Feb 7, 2022 • edited by github-actions bot Loading

codecov bot commented Feb 7, 2022 • edited Loading

Codecov Report

yevhenii-ldv commented Feb 7, 2022 • edited by github-actions bot Loading

yevhenii-ldv commented Feb 7, 2022 • edited by github-actions bot Loading

yevhenii-ldv commented Feb 7, 2022 • edited by github-actions bot Loading

keu Feb 8, 2022 • edited Loading

Choose a reason for hiding this comment

yevhenii-ldv Feb 11, 2022

Choose a reason for hiding this comment

keu left a comment

Choose a reason for hiding this comment

sergei-solonitcyn left a comment

Choose a reason for hiding this comment

yevhenii-ldv commented Feb 10, 2022 • edited by github-actions bot Loading

yevhenii-ldv commented Feb 11, 2022 • edited by github-actions bot Loading

yevhenii-ldv commented Feb 7, 2022 •

edited

Loading

yevhenii-ldv commented Feb 7, 2022 •

edited by github-actions bot

Loading

codecov bot commented Feb 7, 2022 •

edited

Loading

yevhenii-ldv commented Feb 7, 2022 •

edited by github-actions bot

Loading

yevhenii-ldv commented Feb 7, 2022 •

edited by github-actions bot

Loading

yevhenii-ldv commented Feb 7, 2022 •

edited by github-actions bot

Loading

keu Feb 8, 2022 •

edited

Loading

yevhenii-ldv commented Feb 10, 2022 •

edited by github-actions bot

Loading

yevhenii-ldv commented Feb 11, 2022 •

edited by github-actions bot

Loading