-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 Instagram: change requested metrics for stream media_insights
#33889
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
media_insights
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks artem - I left a comment regarding one of the alternate metrics.
Also, does this require a schema refresh?
I think had we caught this on time this would have been a breaking change since customers need to adapt to use the new metrics on any downstream usages of the old metrics, but I'm currently on the fence since it's currently breaking in general, and releasing an immediately required breaking change would pause everyone's syncs (though maybe that's what we need). I'm hoping the answer to these questions helps make that decision.
@@ -358,8 +358,9 @@ def _get_children(self, ids: List): | |||
class MediaInsights(Media): | |||
"""Docs: https://developers.facebook.com/docs/instagram-api/reference/ig-media/insights""" | |||
|
|||
MEDIA_METRICS = ["engagement", "impressions", "reach", "saved"] | |||
CAROUSEL_ALBUM_METRICS = ["carousel_album_engagement", "carousel_album_impressions", "carousel_album_reach", "carousel_album_saved"] | |||
MEDIA_METRICS = ["total_interactions", "impressions", "reach", "saved", "video_views", "likes", "comments", "shares"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@artem1205 I see total_interactions
is listed as an alternative, but the change log states this:
Note: total_interactions, which is listed as an alternative for some of the deprecated metrics, is currently only available using version 18.0 and does not work with older versions. When querying older versions before Dec 11, 2023, please use the engagement metric
This seems to imply total_interactions won't work for us since I believe we're on v11 - is this right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the docs, this should not work with v11, but it does (tested with customers creds).
@pedroslopez , |
What
Resolve https://github.com/airbytehq/oncall/issues/3875
How
change requested metrics for stream
media_insights
See changelog and insight docs for more info
Recommended reading order
airbyte-integrations/connectors/source-instagram/source_instagram/streams.py
🚨 User Impact 🚨
no breaking changes
Pre-merge Actions
Updating a connector
Community member or Airbyter
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.