🎉 Google Ads improvement: Support user-specified queries #5302

vovavovavovavova · 2021-08-10T13:34:58Z

What

Closes #5165
Also this one #5873

How

This allow user to specify the custom GAQL queries as a base for report builder.
However, there are some limitations for now:

* if user wants stream to be incremental, he should not use literals WHERE and ORDER BY. Currently supported format is SELECT a, b,c,d,e ... FROM table. The constructions above should be available for full-refresh streams, but it is not recommended.
* we do not validate queries for now, as well do not check fields compatibility one with another. Those may be done by user manually using
query builder

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

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

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
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions
Connector version bumped like described here

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

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed.

…om_query_stream

vovavovavovavova · 2021-08-10T14:32:55Z

/test connector=connectors/source-google-ads

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

vovavovavovavova · 2021-08-10T14:37:02Z

/test connector=connectors/source-google-ads

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

vovavovavovavova · 2021-08-10T14:47:49Z

/test connector=connectors/source-google-ads

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

vovavovavovavova · 2021-08-10T14:58:07Z

/test connector=connectors/source-google-ads

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

keu · 2021-08-10T16:51:47Z

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

@@ -69,6 +108,11 @@ def streams(self, config: Mapping[str, Any]) -> List[Stream]:
        incremental_stream_config = dict(
            api=google_api, conversion_window_days=config["conversion_window_days"], start_date=config["start_date"]
        )
+
+        custom_query_streams = [


is it possible to validate custom queries in the check_connection method?

Maybe we can use this for validating queries.

used regex in spec.json, since google query validator works on client (UI javascript) side

keu · 2021-08-10T16:51:51Z

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

+
+        custom_query_streams = [
+            CustomQuery(custom_query_config=config["custom_query"][i], **incremental_stream_config)
+            for i in range(len(config.get("custom_query", [])))


why do we need range here???

keu · 2021-08-10T16:52:32Z

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

+        for stream in self.streams(config=config):
+            if not isinstance(stream, (CustomQueryFullRefresh, CustomQueryIncremental)):
+                streams.append(stream.as_airbyte_stream())
+        # TODO: extend with custom defined streams


I'm not sure if you did this here actually, is this still a relevant comment?

just a mark where the default discover logic was extended, will be removed

you don't need to change it at all, just implement this in streams

keu · 2021-08-10T16:53:29Z

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

+            if not isinstance(stream, (CustomQueryFullRefresh, CustomQueryIncremental)):
+                streams.append(stream.as_airbyte_stream())
+        # TODO: extend with custom defined streams
+        for usr_query in config.get("custom_query", []):


why don't you do this inside CustomQueryFullRefresh and CustomQueryIncremental classes ?

we need this to be made in discover, to fill schemas (dynamically, it is very important)

what stops you from doing this in streams??? this is what get_json_schema for

avida · 2021-09-07T11:42:29Z

/test connector=connectors/source-google-ads

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

avida · 2021-09-07T11:53:10Z

/test connector=connectors/source-google-ads

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

sherifnada

looks great! one question on conflicting where clauses

sherifnada · 2021-09-09T04:16:55Z

airbyte-integrations/connectors/source-google-ads/source_google_ads/spec.json

@@ -67,6 +67,27 @@
        "maximum": 1095,
        "default": 14,
        "examples": [14]
+      },
+      "custom_queries": {
+        "type": "array",


Suggested change

"type": "array",

"type": "array",

"title": "Custom GAQL Queries"

sherifnada · 2021-09-09T04:24:01Z

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

+            if google_data_type == "ENUM":
+                field_value = {"type": "string", "enum": list(node.enum_values)}
+            elif google_data_type == "MESSAGE":  # this can be anything (or skip as additionalproperties) ?
+                output_type = ["string", "number", "array", "object", "boolean", "null"]


let's just make it a string and cast it as that

Done, but it took some time, also added test for stream that contains MESSAGE field.

sherifnada · 2021-09-09T04:33:21Z

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

+        result_query = query.replace(columns, new_columns)
+
+        # Modify/insert where condition
+        where_cond = CustomQuery.WHERE_EXPR.search(result_query)


this is a very cool approach, I like it. The only issue I see here is WHERE could contain a time-bound function like LAST_14_DAYS etc. then wouldn't it conflict with this incremental logic?

In this case I think we should not add the where condition and just default to what the user did

In this case I've added checking if custom query contain "segments.date" field on check_connection method. It wont allow adding time-bound conditions to custom queries.

avida · 2021-09-09T08:33:10Z

/test connector=connectors/source-google-ads

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

avida · 2021-09-09T09:25:15Z

/test connector=connectors/source-google-ads

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

avida · 2021-09-10T05:56:16Z

/publish connector=connectors/source-google-ads

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

Issues resolved

vovavovavovavova added 4 commits August 5, 2021 18:23

save the state

7ba083b

save big-big-big draft with dirty pre-upgrade

0b57c89

save draft

7ba9a5c

rm prints and schemas on fly

0b343da

github-actions bot added the area/connectors Connector related issues label Aug 10, 2021

vovavovavovavova added 6 commits August 10, 2021 16:40

fix

cbb274a

Merge remote-tracking branch 'origin/master' into valdemar/#5165_cust…

7c4a212

…om_query_stream

update spec

a4ddc06

cleanup

5a27b95

upgrade configured_catalog for tests

00b9fa1

rm previously used (now not) parameter

6a2d3d7

vovavovavovavova changed the title ~~Valdemar/#5165 custom query stream~~ Google Ads improvement: Support user-specified queries Aug 10, 2021

vovavovavovavova requested review from gaart, keu and vitaliizazmic August 10, 2021 14:33

jrhizor temporarily deployed to more-secrets August 10, 2021 14:35 Inactive

jrhizor temporarily deployed to more-secrets August 10, 2021 14:39 Inactive

vovavovavovavova added 2 commits August 10, 2021 17:42

upd suggestion

7e4c397

format && cleanup imports

2c9efa3

jrhizor temporarily deployed to more-secrets August 10, 2021 14:50 Inactive

remove outdated

b297a6a

jrhizor temporarily deployed to more-secrets August 10, 2021 15:00 Inactive

keu reviewed Aug 10, 2021

View reviewed changes

avida force-pushed the valdemar/#5165_custom_query_stream branch from d8994c9 to 93de86a Compare September 7, 2021 11:32

jrhizor temporarily deployed to more-secrets September 7, 2021 11:35 Inactive

avida force-pushed the valdemar/#5165_custom_query_stream branch 2 times, most recently from ae86f71 to dde02b2 Compare September 7, 2021 11:42

jrhizor temporarily deployed to more-secrets September 7, 2021 11:45 Inactive

avida linked an issue Sep 7, 2021 that may be closed by this pull request

QoL: when test failed post link to gradle run into comments #5873

Closed

avida force-pushed the valdemar/#5165_custom_query_stream branch from dde02b2 to dae0d6f Compare September 7, 2021 11:52

jrhizor temporarily deployed to more-secrets September 7, 2021 11:55 Inactive

Add gradle link on CI comment when build failed.

612ffc3

avida force-pushed the valdemar/#5165_custom_query_stream branch from dae0d6f to 612ffc3 Compare September 7, 2021 12:05

avida requested a review from sherifnada September 7, 2021 14:46

sherifnada reviewed Sep 9, 2021

View reviewed changes

sherifnada self-requested a review September 9, 2021 04:34

jrhizor temporarily deployed to more-secrets September 9, 2021 08:36 Inactive

Fix review comments

22be66e

jrhizor temporarily deployed to more-secrets September 9, 2021 09:27 Inactive

avida changed the title ~~Google Ads improvement: Support user-specified queries~~ 🎉 Google Ads improvement: Support user-specified queries Sep 9, 2021

sherifnada approved these changes Sep 10, 2021

View reviewed changes

jrhizor temporarily deployed to more-secrets September 10, 2021 05:58 Inactive

avida merged commit aa9786d into master Sep 10, 2021

avida deleted the valdemar/#5165_custom_query_stream branch September 10, 2021 06:17

jrhizor mentioned this pull request Sep 16, 2021

Bump Airbyte version from 0.29.17-alpha to 0.29.18-alpha #6125

Merged

vladimir-remar mentioned this pull request Sep 20, 2021

Connector Facebook-Marketing: update insights streams with custom entries for fields, breakdowns and action_breakdowns #4864

Closed

26 tasks

jd-sanders mentioned this pull request Dec 7, 2021

🐛 Source Google Ads: Fix problem of valid custom queries failing. #8567

Closed

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉 Google Ads improvement: Support user-specified queries #5302

🎉 Google Ads improvement: Support user-specified queries #5302

vovavovavovavova commented Aug 10, 2021 •

edited by avida

Loading

vovavovavovavova commented Aug 10, 2021 •

edited by github-actions bot

Loading

vovavovavovavova commented Aug 10, 2021 •

edited by github-actions bot

Loading

vovavovavovavova commented Aug 10, 2021 •

edited by github-actions bot

Loading

vovavovavovavova commented Aug 10, 2021 •

edited by github-actions bot

Loading

keu Aug 10, 2021

vitaliizazmic Aug 11, 2021

vovavovavovavova Aug 13, 2021 •

edited

Loading

keu Aug 10, 2021

keu Aug 10, 2021

vovavovavovavova Aug 11, 2021

keu Aug 11, 2021

keu Aug 10, 2021

vovavovavovavova Aug 10, 2021 •

edited

Loading

keu Aug 11, 2021

avida commented Sep 7, 2021 •

edited by github-actions bot

Loading

avida commented Sep 7, 2021 •

edited by github-actions bot

Loading

sherifnada left a comment

sherifnada Sep 9, 2021

sherifnada Sep 9, 2021

avida Sep 9, 2021

sherifnada Sep 9, 2021

avida Sep 9, 2021

avida commented Sep 9, 2021 •

edited by github-actions bot

Loading

avida commented Sep 9, 2021 •

edited by github-actions bot

Loading

avida commented Sep 10, 2021 •

edited by github-actions bot

Loading

	"type": "array",
	"type": "array",
	"title": "Custom GAQL Queries"

🎉 Google Ads improvement: Support user-specified queries #5302

🎉 Google Ads improvement: Support user-specified queries #5302

Conversation

vovavovavovavova commented Aug 10, 2021 • edited by avida Loading

What

How

Recommended reading order

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

vovavovavovavova commented Aug 10, 2021 • edited by github-actions bot Loading

vovavovavovavova commented Aug 10, 2021 • edited by github-actions bot Loading

vovavovavovavova commented Aug 10, 2021 • edited by github-actions bot Loading

vovavovavovavova commented Aug 10, 2021 • edited by github-actions bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vovavovavovavova Aug 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vovavovavovavova Aug 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avida commented Sep 7, 2021 • edited by github-actions bot Loading

avida commented Sep 7, 2021 • edited by github-actions bot Loading

sherifnada left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avida commented Sep 9, 2021 • edited by github-actions bot Loading

avida commented Sep 9, 2021 • edited by github-actions bot Loading

avida commented Sep 10, 2021 • edited by github-actions bot Loading

vovavovavovavova commented Aug 10, 2021 •

edited by avida

Loading

vovavovavovavova commented Aug 10, 2021 •

edited by github-actions bot

Loading

vovavovavovavova commented Aug 10, 2021 •

edited by github-actions bot

Loading

vovavovavovavova commented Aug 10, 2021 •

edited by github-actions bot

Loading

vovavovavovavova commented Aug 10, 2021 •

edited by github-actions bot

Loading

vovavovavovavova Aug 13, 2021 •

edited

Loading

vovavovavovavova Aug 10, 2021 •

edited

Loading

avida commented Sep 7, 2021 •

edited by github-actions bot

Loading

avida commented Sep 7, 2021 •

edited by github-actions bot

Loading

avida commented Sep 9, 2021 •

edited by github-actions bot

Loading

avida commented Sep 9, 2021 •

edited by github-actions bot

Loading

avida commented Sep 10, 2021 •

edited by github-actions bot

Loading