Skip to content
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

🐛 Source google ads: mark custom query fields required #15858

Merged
merged 11 commits into from
Sep 15, 2022

Conversation

tuliren
Copy link
Contributor

@tuliren tuliren commented Aug 22, 2022

What

@github-actions github-actions bot added the area/connectors Connector related issues label Aug 22, 2022
@tuliren tuliren requested a review from a team August 22, 2022 21:08
@tuliren
Copy link
Contributor Author

tuliren commented Aug 22, 2022

I am not 100% sure this is the correct solution. Will pause until getting some review from the API team first.

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Aug 24, 2022
@tuliren
Copy link
Contributor Author

tuliren commented Aug 24, 2022

/test connector=connectors/source-google-ads

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

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 FAILED integration_tests/test_incremental.py::test_incremental_sync - KeyErro...
	 �[31m========================= �[31m�[1m1 failed�[0m, �[32m1 passed�[0m�[31m in 15.56s�[0m�[31m =========================�[0m

Copy link
Contributor

@alafanechere alafanechere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tuliren, this introduces a backward incompatibility that will be detected by SAT: adding required fields makes the previous connector config incompatible.
In this situation, if I understood the problem well, having empty query or table_name is causing a problem, so I think we can bypass backward compatible tests. I'll commit changes to the acceptance-test-config.yml to bypass backward compatibility tests.

@tuliren
Copy link
Contributor Author

tuliren commented Sep 5, 2022

/test connector=connectors/source-google-ads

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

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 FAILED integration_tests/test_incremental.py::test_incremental_sync - KeyErro...
	 �[31m========================= �[31m�[1m1 failed�[0m, �[32m1 passed�[0m�[31m in 12.91s�[0m�[31m =========================�[0m

@sherifnada
Copy link
Contributor

@tuliren you should assign this as an OC to GL, seems like there is a non-trivial issue here maybe

@tuliren
Copy link
Contributor Author

tuliren commented Sep 6, 2022

@sherifnada, yes, I realized that. Will do.

@davydov-d
Copy link
Collaborator

davydov-d commented Sep 14, 2022

/test connector=connectors/source-google-ads

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

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestSpec::test_backward_compatibility[inputs0] - source_...
FAILED test_full_refresh.py::TestFullRefresh::test_sequential_reads[inputs0]
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestIncremental.test_two_sequential_reads because not found in the config
============= 2 failed, 26 passed, 1 skipped in 1612.67s (0:26:52) =============

@davydov-d
Copy link
Collaborator

davydov-d commented Sep 15, 2022

/test connector=connectors/source-google-ads

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

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_full_refresh.py::TestFullRefresh::test_sequential_reads[inputs0]
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestIncremental.test_two_sequential_reads because not found in the config
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:57: Backward compatibility tests are disabled for version 0.1.45.
============= 1 failed, 26 passed, 2 skipped in 1556.99s (0:25:56) =============

@davydov-d
Copy link
Collaborator

davydov-d commented Sep 15, 2022

/test connector=connectors/source-google-ads

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

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 FAILED integration_tests/test_incremental.py::test_incremental_sync - KeyErro...
	 �[31m========================= �[31m�[1m1 failed�[0m, �[32m1 passed�[0m�[31m in 3.35s�[0m�[31m ==========================�[0m

@davydov-d
Copy link
Collaborator

davydov-d commented Sep 15, 2022

/test connector=connectors/source-google-ads

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

Name                                       Stmts   Miss  Cover
--------------------------------------------------------------
source_google_ads/models.py                   18      0   100%
source_google_ads/__init__.py                  2      0   100%
source_google_ads/google_ads.py               68     10    85%
source_google_ads/streams.py                 163     26    84%
source_google_ads/source.py                   86     25    71%
source_google_ads/custom_query_stream.py      75     46    39%
--------------------------------------------------------------
TOTAL                                        412    107    74%
Name                                       Stmts   Miss  Cover
--------------------------------------------------------------
source_google_ads/models.py                   18      0   100%
source_google_ads/__init__.py                  2      0   100%
source_google_ads/streams.py                 163      8    95%
source_google_ads/source.py                   86      5    94%
source_google_ads/custom_query_stream.py      75      6    92%
source_google_ads/google_ads.py               68     12    82%
--------------------------------------------------------------
TOTAL                                        412     31    92%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          10      4    60%   15-18
	 source_acceptance_test/config.py                        83      6    93%   78-80, 84-86
	 source_acceptance_test/conftest.py                     164    164     0%   6-282
	 source_acceptance_test/plugin.py                        48     48     0%   6-104
	 source_acceptance_test/tests/test_core.py              329    111    66%   39, 50-58, 63-70, 74-75, 79-80, 164, 202-219, 228-236, 240-245, 251, 284-289, 327-334, 374-376, 379, 439-448, 477-478, 484, 487, 520-530, 543-568, 573-577
	 source_acceptance_test/tests/test_full_refresh.py       52      2    96%   34, 65
	 source_acceptance_test/tests/test_incremental.py       121     25    79%   21-23, 29-31, 36-43, 48-61, 208-216
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     17    78%   15-16, 24-30, 47-54, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       110     48    56%   23-26, 32, 36, 39-64, 67-69, 72-74, 77-79, 82-84, 87-89, 92-110, 144-146
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1325    463    65%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestIncremental.test_two_sequential_reads because not found in the config
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:57: Backward compatibility tests are disabled for version 0.1.45.
================== 27 passed, 2 skipped in 614.92s (0:10:14) ===================

@davydov-d
Copy link
Collaborator

davydov-d commented Sep 15, 2022

/publish connector=connectors/source-google-ads

🕑 Publishing the following connectors:
connectors/source-google-ads
https://github.com/airbytehq/airbyte/actions/runs/3059805922


Connector Did it publish? Were definitions generated?
connectors/source-google-ads

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@davydov-d davydov-d merged commit 2e17e3f into master Sep 15, 2022
@davydov-d davydov-d deleted the liren/oncall-467-add-required-fields branch September 15, 2022 11:25
letiescanciano added a commit that referenced this pull request Sep 15, 2022
* master: (200 commits)
  🪟 🧹 Display returned error messages on replication view (#16280)
  🎉 Source mixpanel: Use "Retry-After" header for backoff (#16770)
  🐛 Source google ads: mark custom query fields required (#15858)
  🪟 🔧Remove useRouter hook (#16598)
  CDK: improve TypeTransformer to convert simple types to array of simple types (#16636)
  CDK: TypeTransformer - warning message more informative (#16695)
  Source MySQL: Add Python SAT to detect backwards breaking changes (#16445)
  remove eager (#16756)
  bump com.networknt:json-schema-validator to latest version (#16619)
  Remove Cloud from Kafka docs (#16753)
  Normalization Summaries table and read/write methods (#16655)
  comment out flaky test suite while it is being investigated (#16752)
  Update ConfigRepository to read protocol version (#16670)
  Use LOG4J2 to wrap connectors logs to JSON format (#15668)
  Update connector catalog (#16749)
  🪟 🎨 Remove feedback modal from UI (#16548)
  Add missing env var for Kube overlays (#16747)
  Prepare for React v18 upgrade (#16694)
  🪟 🐛 Fix direct job linking to work with pagination (#16517)
  Fix formatting (#16743)
  ...
robbinhan pushed a commit to robbinhan/airbyte that referenced this pull request Sep 29, 2022
* Mark query and table_name as required

* Bump version

* bypass backward compat tests

* bypass backward compat tests

* revert change on source-google-sheets/acceptance-test-config

* oncall 467 - source google ads: fix sats

* oncall airbytehq#467 source google-ads: one more SATs fix

* auto-bump connector version [ci skip]

Co-authored-by: alafanechere <augustin.lafanechere@gmail.com>
Co-authored-by: Denys Davydov <davydov.den18@gmail.com>
Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
* Mark query and table_name as required

* Bump version

* bypass backward compat tests

* bypass backward compat tests

* revert change on source-google-sheets/acceptance-test-config

* oncall 467 - source google ads: fix sats

* oncall airbytehq#467 source google-ads: one more SATs fix

* auto-bump connector version [ci skip]

Co-authored-by: alafanechere <augustin.lafanechere@gmail.com>
Co-authored-by: Denys Davydov <davydov.den18@gmail.com>
Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants