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

Google ads added fields and reports #9310

Merged
merged 10 commits into from
Apr 19, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
"ad_group_ad.ad.legacy_responsive_display_ad.accent_color": {
"type": ["null", "string"]
},
"ad_group.id": {
"type": ["null", "string"]
Copy link
Contributor

Choose a reason for hiding this comment

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

@schlattk type must be "integer", not "string".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@augan-rymkhan let me know if any further changes are needed

},
"customer.currency_code": {
"type": ["null", "string"]
},
Expand Down Expand Up @@ -171,6 +174,9 @@
"segments.day_of_week": {
"type": ["null", "string"]
},
"segments.keyword.ad_group_criterion": {
"type": ["null", "string"]
},
"ad_group_ad.ad.expanded_text_ad.description": {
"type": ["null", "string"]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,21 @@
"campaign.video_brand_safety_suitability": {
"type": ["null", "string"]
},
"metrics.clicks": {
"type": ["null", "integer"]
},
"metrics.cost_micros": {
"type": ["null", "integer"]
},
"metrics.impressions": {
"type": ["null", "number"]
},
"metrics.video_views": {
"type": ["null", "number"]
},
"metrics.video_quartile_p100_rate": {
"type": ["null", "number"]
},
"segments.date": {
"type": ["null", "string"],
"format": "date"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,24 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"type": "object",
"properties": {
"ad_group.name": {
"type": ["null", "string"]
},
"click_view.gclid": {
"type": ["null", "string"]
},
"click_view.ad_group_ad": {
"type": ["null", "string"]
},
"click_view.keyword": {
"type": ["null", "string"]
},
"click_view.keyword_info.match_type": {
"type": ["null", "string"]
},
"click_view.keyword_info.text": {
"type": ["null", "string"]
},
"campaign.id": {
"type": ["null", "integer"]
},
Expand All @@ -19,6 +34,9 @@
},
"campaign.name": {
"type": ["null", "string"]
},
"segments.ad_network_type": {
"type": ["null", "string"]
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,6 @@
"metrics.ctr": {
"type": ["null", "number"]
},
"segments.date": {
"type": ["null", "string"],
"format": "date"
},
"segments.day_of_week": {
"type": ["null", "string"]
},
Expand Down Expand Up @@ -210,6 +206,9 @@
"ad_group_criterion.tracking_url_template": {
"type": ["null", "string"]
},
"ad_group_criterion.keyword.match_type": {
"type": ["null", "string"]
},
"ad_group_criterion.url_custom_parameters": {
"type": ["null", "array"],
"items": {
Expand Down Expand Up @@ -248,6 +247,9 @@
},
"segments.year": {
"type": ["null", "integer"]
},
"segments.date": {
"type": ["null", "integer"]
Copy link
Contributor

Choose a reason for hiding this comment

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

@schlattk Why segments.date type is changed from "string" to "integer" and removed "format": "date" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@augan-rymkhan ok thanks if you could have another look

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,24 @@
"segments.date": {
"type": ["null", "string"],
"format": "date"
},
"campaign.bidding_strategy_type": {
"type": ["null", "string"]
},
"metrics.clicks": {
"type": ["null", "integer"]
},
"metrics.cost_micros": {
"type": ["null", "integer"]
},
"metrics.impressions": {
"type": ["null", "integer"]
},
"ad_group_criterion.criterion_id": {
"type": ["null", "integer"]
},
"segments.ad_network_type": {
Copy link
Contributor

Choose a reason for hiding this comment

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

@schlattk Error happens because this field was added.

message: "Cannot select the following segments because at least one unsupported metric is found in SELECT or WHERE clause: \'segments.ad_network_type\'(unsupported metrics: \'historical_quality_score\')."\n}\nrequest_id: "hBx3G90_bSZ-rVpnBMQlxQ"\n, 'hBx3G90_bSZ-rVpnBMQlxQ')"}}

Copy link
Contributor

Choose a reason for hiding this comment

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

It works when read command is run without this field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@augan-rymkhan ok but it is a supported field according to documentation and also it has been running live for a long time without any errors. What do you suggest is the solution?

Copy link
Contributor

@augan-rymkhan augan-rymkhan Feb 28, 2022

Choose a reason for hiding this comment

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

@schlattk It seems we can not query metrics.historical_quality_score and segments.ad_network_type together in keyword_report stream.

In docs:
A segment in the SELECT clause is incompatible with a metric in the SELECT or WHERE clause.
For more you can read PROHIBITED_SEGMENT_WITH_METRIC_IN_SELECT_OR_WHERE_CLAUSE.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes @augan-rymkhan that appears to be the problem. Ok, I removed the field I think we can most probably do without it in this report.

"type": ["null", "integer"]
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
ClickView,
DisplayKeywordPerformanceReport,
DisplayTopicsPerformanceReport,
KeywordView,
Copy link
Contributor

Choose a reason for hiding this comment

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

@schlattk stream KeywordView is not defined anywhere. You should remove all the references to KeywordView (resolving merge conflicts also), because this stream is implemented as KeywordReport.

GeographicReport,
KeywordReport,
ShoppingPerformanceReport,
Expand Down Expand Up @@ -111,11 +112,21 @@ def streams(self, config: Mapping[str, Any]) -> List[Stream]:
incremental_stream_config = self.get_incremental_stream_config(google_api, config, tz=time_zone)

streams = [
<<<<<<< HEAD
AdGroupAds(api=google_api),
AdGroups(api=google_api),
Accounts(api=google_api),
=======
AdGroupAds(**incremental_stream_config),
AdGroups(**incremental_stream_config),
Accounts(**incremental_stream_config),
>>>>>>> master
Campaigns(**incremental_stream_config),
augan-rymkhan marked this conversation as resolved.
Show resolved Hide resolved
ClickView(**incremental_stream_config),
<<<<<<< HEAD
KeywordView(**incremental_stream_config)
Copy link
Contributor

Choose a reason for hiding this comment

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

No need in KeywordView.

] + custom_query_streams
=======
]

custom_query_streams = [
Expand All @@ -139,3 +150,4 @@ def streams(self, config: Mapping[str, Any]) -> List[Stream]:
]
)
return streams
>>>>>>> master