Add the new Facebook API v10.0 fields to the ads_insights schema. #3646
Conversation
|
Hey @sherifnada, I iterated on this and narrowed it down to find the offending fields—the API was rejecting "unique_conversions" and "cost_per_unique_conversion". It turns out I was using the wrong webpage as a reference for the available fields. The correct reference is: https://developers.facebook.com/docs/marketing-api/reference/ads-insights#fields I've removed the offending fields, and added the rest of the fields that are supported. I've confirmed using my script that the Ads Insights API does accept a request for exactly this set of fields and works properly. |
…rsions" fields and add other available fields that have been tested and confirmed to work.
zestyping
force-pushed
the
ping/facebook-ads-insights-v10-schema
branch
from
5c2a52b
to
4d19cb2
Compare
|
Rebased on master. |
|
/test connector=source-facebook-marketing
|
… (#3693) * Add all the missing fields except AdAsset and AdInsightsResult fields. * Sort and format ad_insights.json with json.dumps(..., indent=2, sort_keys=True). * Remove fields from ads_insights.json that have not been externally tested. * Facebook source: Remove the offending "unique_conversions" and "conversions" fields and add other available fields that have been tested and confirmed to work. * Update all the other ads_insights schemas for the various breakdowns. * format * bump version, update changelog * fix async job handling * fix async job handling, add missing dockerignore * replace platform_and_device stream in integration tests because it fails Co-authored-by: Ka-Ping Yee <zestyping@gmail.com> Co-authored-by: Eugene Kulak <kulak.eugene@gmail.com>
|
@zestyping The new version of connector (0.2.7) was released yesterday, please keep in mind that closing this PR |
What
This adds almost all of the missing fields to the ads_insights schema, to match the list of fields documented here:
https://developers.facebook.com/docs/marketing-api/reference/adgroup/insights/#fields
How
I edited the JSON file by hand, then sorted the keys and reviewed it by comparing it to the documentation page by hand. Note that I made these edits only by pattern-matching; I don't actually have a deeper understanding of the syntax.
I applied the following heuristics:
["null", "integer"]for all fields described as "numeric string" whose name ended in "clicks"["null", "number"]for all other fields described as "numeric string"["null", "string"]for all other fields described as "string"List<AdsActionStats>but had nested types declared in the original file, I replaced the nested type with a reference toads_action_stats.json(consistent with the rest of theList<AdsActionStats>fields.I did not add the following fields, which are of types that haven't been defined in the
shareddirectory yet:comparison_node(AdsInsightsComparison)dda_results(List<AdsInsightsResult>)description_asset(AdAssetDescription)image_asset(AdAssetImage)rule_asset(AdAssetRule)title_asset(AdAssetTitle)video_asset(AdAssetVideo)Pre-merge Checklist
I have not tested anything.
Reviewer notes
This change is divided into two commits. The first commit has the new fields added, while preserving the rest of the original file so you can see a minimal diff. The second commit has the whole file sorted by
json.dumps(completely automated, with no manual edits), so you can review it against the API documentation.