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

🎉 Source Facebook Marketing: Add the option to fetch thumbnail image data. #8649

Merged
merged 15 commits into from
Dec 17, 2021

Conversation

zestyping
Copy link
Contributor

What

The Facebook ad_creatives stream contains a thumbnail_url field that points to a thumbnail image of the ad image or ad video. However, the URL in that field is only valid for a short time; it expires after a few days.

This change allows you to optionally fetch the contents of the thumbnail image together with the ad_creatives stream. The resulting data is encoded as a data: URL and placed in the new thumbnail_data_url field so you can easily use it as an image source URL.

How

  1. There's a new fetch_thumbnail_images setting in the Facebook Marketing source config.
  2. When this setting is enabled, the AdCreatives stream post-processes the retrieved record by fetching the thumbnail_url (using the requests library) and encodes the data as a data: URL, which it stores in the thumbnail_data_url field.
  3. A thumbnail_data_url field is added to the schema for the ad creatives stream, so that it can be returned in the Airbyte data. However, this field is excluded when requesting data from Facebook, because it's not a field that Facebook knows about.

🚨 User Impact 🚨

There are no backward-incompatible changes; upgrading to this version of the Facebook Marketing source should have no effect on existing connections. The fetch_thumbnail_images setting defaults to False, and the new behaviour is only enabled when the user changes the setting to True.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

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
    • Connector's bootstrap.md. See description and examples
    • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • PR name follows PR naming conventions

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
  • After the new connector version is published, connector version bumped in the seed directory as described here
  • Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

@github-actions github-actions bot added the area/connectors Connector related issues label Dec 8, 2021
@marcosmarxm
Copy link
Member

@misteryeo can you review the UX change this code makes?

Copy link
Contributor

@misteryeo misteryeo left a comment

Choose a reason for hiding this comment

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

This LGTM - not a blocker but I'd be curious on my comment about documentation to see if we can hyperlink as part of the config description as I imagine not everyone is familiar with the concept outlined there.

@@ -76,7 +76,9 @@ class Config:
default_factory=pendulum.now,
)

include_deleted: bool = Field(default=False, description="Include data from deleted campaigns, ads, and adsets.")
fetch_thumbnail_images: bool = Field(default=False, description="In each Ad Creative, fetch the thumbnail_url and store the result in thumbnail_data_url")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any documentation that we can link to from Facebook that demonstrates how this works?

Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

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

thanks @zestyping

@CLAassistant
Copy link

CLAassistant commented Dec 17, 2021

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added area/documentation Improvements or additions to documentation area/frontend area/platform issues related to the platform area/scheduler area/server area/worker Related to worker CDK Connector Development Kit kubernetes normalization labels Dec 17, 2021
@marcosmarxm marcosmarxm temporarily deployed to more-secrets December 17, 2021 19:55 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets December 17, 2021 19:59 Inactive
@marcosmarxm marcosmarxm temporarily deployed to more-secrets December 17, 2021 20:12 Inactive
@github-actions github-actions bot removed area/platform issues related to the platform area/frontend labels Dec 17, 2021
@marcosmarxm marcosmarxm temporarily deployed to more-secrets December 17, 2021 22:22 Inactive
@marcosmarxm marcosmarxm merged commit 9acfc81 into airbytehq:master Dec 17, 2021
@zestyping
Copy link
Contributor Author

zestyping commented Dec 17, 2021 via email

schlattk pushed a commit to schlattk/airbyte that referenced this pull request Jan 4, 2022
…data. (airbytehq#8649)

* feat(zendesk): Add Brands and CustomRoles

* feat(zendesk): add incremental unsorted cursor stream
implement IncrementalUnsortedCursorStream to ticket_metrics

* feat(zendesk): use sorted cursor pagination
for ticket comments and macros

* feat(zendesk): use unsorted cursor stream
for groups, group memberships and satisfaction ratings

* fix(zendesk): use safe method to get value from nested dict

* style(zendesk): reformat using gradlew

* fix(zendesk): format created_at and updated_at to date-time format

* feat(zendesk): add business hours schedule

* bump connector version

* bump dockerfile version

* reset

* resolve webapp files

Co-authored-by: asyarif93 <asyarif93@gmail.com>
Co-authored-by: Marcos Marx <marcosmarxm@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants