Fix(Source-Facebook-Marketing): Update breakdown schema for `BaseInsightStream` #55760

agarctfi · 2025-03-13T23:06:00Z

What

Solves: https://github.com/airbytehq/oncall/issues/7624

How

Added missing schema & unit tests to ensure all breakdowns are added

Review guide

Check schema datatypes. One doubt I have is whether we should leave them all as Strings in this context or have the actual datatype for the schema recorded. I'm not too sure since this is used in the configuration of an array of Strings, but I'm not sure if it is used elsewhere.

User Impact

Can this PR be safely reverted and rolled back?

YES 💚
NO ❌

vercel · 2025-03-13T23:06:05Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 20, 2025 3:08pm

agarctfi · 2025-03-13T23:07:07Z

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (1cfa4fe)

…ightsSchemaFix

brianjlai

makes sense overall! just question about how we found the list of properties to add

brianjlai · 2025-03-18T23:49:14Z

...ors/source-facebook-marketing/source_facebook_marketing/schemas/ads_insights_breakdowns.json

@@ -163,6 +167,126 @@
        "video_name": { "type": ["null", "string"] },
        "id": { "type": ["null", "string"] }
      }
+    },
+    "breakdown_reporting_ad_id": {


i don't know salesforce all too well, but where were some of these types generated from?

I see things like mmm and fidelity_type are found in:

from facebook_business.adobjects.adsinsights import AdsInsights AdsInsights.Breakdowns.__dict__

but some of these aren't in that dictionary so I'm curious where those came from. Also it would help me validate that the types are correct if I can compare them against FBCA

These come from the facebok_business library.

class Breakdowns: ad_format_asset = 'ad_format_asset' age = 'age' app_id = 'app_id' body_asset = 'body_asset' breakdown_reporting_ad_id = 'breakdown_reporting_ad_id' call_to_action_asset = 'call_to_action_asset' coarse_conversion_value = 'coarse_conversion_value' conversion_destination = 'conversion_destination' country = 'country' description_asset = 'description_asset' device_platform = 'device_platform' dma = 'dma' fidelity_type = 'fidelity_type' frequency_value = 'frequency_value' gender = 'gender' hourly_stats_aggregated_by_advertiser_time_zone = 'hourly_stats_aggregated_by_advertiser_time_zone' hourly_stats_aggregated_by_audience_time_zone = 'hourly_stats_aggregated_by_audience_time_zone' hsid = 'hsid' image_asset = 'image_asset' impression_device = 'impression_device' is_conversion_id_modeled = 'is_conversion_id_modeled' is_rendered_as_delayed_skip_ad = 'is_rendered_as_delayed_skip_ad' landing_destination = 'landing_destination' link_url_asset = 'link_url_asset' marketing_messages_btn_name = 'marketing_messages_btn_name' mdsa_landing_destination = 'mdsa_landing_destination' media_asset_url = 'media_asset_url' media_creator = 'media_creator' media_destination_url = 'media_destination_url' media_format = 'media_format' media_origin_url = 'media_origin_url' media_text_content = 'media_text_content' media_type = 'media_type' mmm = 'mmm' place_page_id = 'place_page_id' platform_position = 'platform_position' postback_sequence_index = 'postback_sequence_index' product_id = 'product_id' publisher_platform = 'publisher_platform' redownload = 'redownload' region = 'region' signal_source_bucket = 'signal_source_bucket' skan_campaign_id = 'skan_campaign_id' skan_conversion_id = 'skan_conversion_id' skan_version = 'skan_version' sot_attribution_model_type = 'sot_attribution_model_type' sot_attribution_window = 'sot_attribution_window' sot_channel = 'sot_channel' sot_event_type = 'sot_event_type' sot_source = 'sot_source' standard_event_content_type = 'standard_event_content_type' title_asset = 'title_asset' user_persona_id = 'user_persona_id' user_persona_name = 'user_persona_name' video_asset = 'video_asset'

They are tested here, and are defined here.

@agarctfi ah yeah okay, my bad, I was reviewing locally and I had not updated facebook's python dependencies in a while so I was looking at my Breakdown list from an older version which was missing some of the new ones.

One last question here, I noticed you pulled the id fields from existing fields into separate body_asset_id, call_to_action_asset_id, description_asset_id. Was there a bug or functional reason for doing this?

@brianjlai, I think those were already there, and not too sure what their functional reasons are. I ran a script I made to alphabetize the schema, which moved some around, but the final output should be what was already included in the schema prior, and whatever was missing from this is the Breakdowns class.

sorry, maybe my wording was confusing, but in the Breakdowns list above there is no body_asset_id. but in the schema now I see:

"body_asset_id": { "description": "The ID of the main content or message of the ad.", "type": ["null", "string"] },

And so we have 3 extra fields that are prefixed with _id and I wasn't sure why we needed those when there is already an existing: body_asset, call_to_action_asset and description_asset already in the schema.

Got it! I can remove them; I was just saying that they were already there (in master); I think maybe they are outdated Breakdowns of older versions of the facebok_business library.

Well, after searching, though, I didn't see that facebook_marketing even used these fields, but we do use them here:

airbyte/airbyte-integrations/connectors/source-facebook-marketing/source_facebook_marketing/streams/base_insight_streams.py

Lines 48 to 56 in 83ecbe0

object_breakdowns = {

"body_asset": "body_asset_id",

"call_to_action_asset": "call_to_action_asset_id",

"description_asset": "description_asset_id",

"image_asset": "image_asset_id",

"link_url_asset": "link_url_asset_id",

"title_asset": "title_asset_id",

"video_asset": "video_asset_id",

}

From this PR, to solve this issue.

I think this here gives some real insight and seems to be something we did intentionally to avoid breaking changes, but it seems like it was already done previously (old code), so we left it then. So it may be best to leave it, but I would love to hear your input on it @brianjlai

yeah i agree, i think my curiosity is getting the better of me and maybe it's best not to go further down the rabbit hole. and agree on not making this breaking. Maybe we have some use for it maybe we don't but I'm leaving as is. thanks for investigating further. I'm good to merge this

...ors/source-facebook-marketing/source_facebook_marketing/schemas/ads_insights_breakdowns.json

agarctfi · 2025-03-19T00:18:08Z

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (5bb5dd4)

brianjlai

approving, but are you able to get a regression test run before the merge?

…ightsSchemaFix

agarctfi · 2025-03-20T23:55:40Z

Regressions timeout, but I ran manually on my cloud env via dev image, and they look good.

We also pinned the user, and it did resolve the issue for them!

agarctfi · 2025-03-21T14:19:59Z

/approve-regression-tests

Check job output.

✅ Approving regression tests

…ghtStream` (airbytehq#55760) Co-authored-by: Octavia Squidington III <[email protected]> Co-authored-by: Natik Gadzhi <[email protected]>

Fix(Source-Facebook-Marketing): Update breakdown schema for

1226a7a

agarctfi requested a review from a team as a code owner March 13, 2025 23:06

agarctfi changed the title ~~Fix(Source-Facebook-Marketing): Update breakdown schema for~~ Fix(Source-Facebook-Marketing): Update breakdown schema for BaseInsightStream Mar 13, 2025

octavia-squidington-iii added the connectors/source/facebook-marketing label Mar 13, 2025

Update Doc

e4978b7

chore: auto-fix lint and format issues

1cfa4fe

vercel bot deployed to Preview March 13, 2025 23:17 View deployment

Merge branch 'master' into agarctfi/source-facebook-marketing/baseIns…

fd27274

…ightsSchemaFix

vercel bot deployed to Preview March 16, 2025 15:11 View deployment

Merge branch 'master' into agarctfi/source-facebook-marketing/baseIns…

9a9ba7d

…ightsSchemaFix

vercel bot deployed to Preview March 18, 2025 21:18 View deployment

brianjlai reviewed Mar 18, 2025

View reviewed changes

alphabetize schema

bf72d8c

chore: auto-fix lint and format issues

5bb5dd4

vercel bot deployed to Preview March 19, 2025 00:28 View deployment

brianjlai approved these changes Mar 19, 2025

View reviewed changes

Merge branch 'master' into agarctfi/source-facebook-marketing/baseIns…

14ad720

…ightsSchemaFix

vercel bot deployed to Preview March 19, 2025 22:55 View deployment

Merge branch 'master' into agarctfi/source-facebook-marketing/baseIns…

07f99bc

…ightsSchemaFix

vercel bot deployed to Preview March 20, 2025 15:08 View deployment

agarctfi merged commit 05cf22d into master Mar 21, 2025
31 of 32 checks passed

agarctfi deleted the agarctfi/source-facebook-marketing/baseInsightsSchemaFix branch March 21, 2025 14:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix(Source-Facebook-Marketing): Update breakdown schema for `BaseInsightStream` #55760

Fix(Source-Facebook-Marketing): Update breakdown schema for `BaseInsightStream` #55760

agarctfi commented Mar 13, 2025

vercel bot commented Mar 13, 2025 •

edited

Loading

agarctfi commented Mar 13, 2025 •

edited by github-actions bot

Loading

brianjlai left a comment

brianjlai Mar 18, 2025

agarctfi Mar 19, 2025 •

edited

Loading

brianjlai Mar 19, 2025

agarctfi Mar 19, 2025

brianjlai Mar 19, 2025

agarctfi Mar 19, 2025

brianjlai Mar 19, 2025

agarctfi commented Mar 19, 2025 •

edited by github-actions bot

Loading

brianjlai left a comment

agarctfi commented Mar 20, 2025

agarctfi commented Mar 21, 2025 •

edited by github-actions bot

Loading

	object_breakdowns = {
	"body_asset": "body_asset_id",
	"call_to_action_asset": "call_to_action_asset_id",
	"description_asset": "description_asset_id",
	"image_asset": "image_asset_id",
	"link_url_asset": "link_url_asset_id",
	"title_asset": "title_asset_id",
	"video_asset": "video_asset_id",
	}

Fix(Source-Facebook-Marketing): Update breakdown schema for BaseInsightStream #55760

Fix(Source-Facebook-Marketing): Update breakdown schema for BaseInsightStream #55760

Conversation

agarctfi commented Mar 13, 2025

What

How

Review guide

User Impact

Can this PR be safely reverted and rolled back?

vercel bot commented Mar 13, 2025 • edited Loading

agarctfi commented Mar 13, 2025 • edited by github-actions bot Loading

brianjlai left a comment

Choose a reason for hiding this comment

brianjlai Mar 18, 2025

Choose a reason for hiding this comment

agarctfi Mar 19, 2025 • edited Loading

Choose a reason for hiding this comment

brianjlai Mar 19, 2025

Choose a reason for hiding this comment

agarctfi Mar 19, 2025

Choose a reason for hiding this comment

brianjlai Mar 19, 2025

Choose a reason for hiding this comment

agarctfi Mar 19, 2025

Choose a reason for hiding this comment

brianjlai Mar 19, 2025

Choose a reason for hiding this comment

agarctfi commented Mar 19, 2025 • edited by github-actions bot Loading

brianjlai left a comment

Choose a reason for hiding this comment

agarctfi commented Mar 20, 2025

agarctfi commented Mar 21, 2025 • edited by github-actions bot Loading

Fix(Source-Facebook-Marketing): Update breakdown schema for `BaseInsightStream` #55760

Fix(Source-Facebook-Marketing): Update breakdown schema for `BaseInsightStream` #55760

vercel bot commented Mar 13, 2025 •

edited

Loading

agarctfi commented Mar 13, 2025 •

edited by github-actions bot

Loading

agarctfi Mar 19, 2025 •

edited

Loading

agarctfi commented Mar 19, 2025 •

edited by github-actions bot

Loading

agarctfi commented Mar 21, 2025 •

edited by github-actions bot

Loading