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.

Sign up for GitHub

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

Jump to bottom

🎉 Source Facebook Marketing: Allow configuring insights lookback window #3396

Merged
merged 9 commits into from
May 20, 2021
Merged

🎉 Source Facebook Marketing: Allow configuring insights lookback window #3396

merged 9 commits into from
May 20, 2021

Conversation

po3na4skld
Copy link
Contributor

@po3na4skld po3na4skld commented May 13, 2021 •
edited
Loading

What

Adding new filed to customize in facebook marketing api #2920

How

Updated spec.json file with new fileld settings and changed init method in Client and AdsInsightApi

Pre-merge Checklist

  • Run integration tests
  • Publish Docker images

Recommended reading order

  1. spec.json
  2. client.py
  3. api.py

sherifnada
sherifnada suggested changes May 13, 2021
View reviewed changes
airbyte-integrations/connectors/source-facebook-marketing/source_facebook_marketing/spec.json
@@ -26,6 +26,14 @@
"type": "boolean",
"description": "Include data from deleted campaigns, ads, and adsets.",
"default": "false"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

this is the first step, but this needs to be passed into the connector so it would actually use this value.

Also, could you add a test using a mocked HTTP request to verify this works?

@sherifnada sherifnada requested review from yevhenii-ldv and removed request for davinchia May 13, 2021 21:44
@po3na4skld po3na4skld linked an issue May 14, 2021 that may be closed by this pull request
Allow configuring Facebook Marketing insights buffer days #2920
Closed
@sherifnada sherifnada changed the title added new field 🎉 Source Facebook Marketing: Allow configuring insights lookback window May 14, 2021
@po3na4skld
gradlew changes
b8e782c
@po3na4skld
gradlew changes
8de7dee
@po3na4skld
Merge branch 'master' of github.com:airbytehq/airbyte into 2920-Allow…
7b39c0f 
…-configuring-Facebook-Marketing-insights-buffer-days

To catch up master changes
sherifnada
sherifnada approved these changes May 19, 2021
View reviewed changes
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

LGTM! Feel free to bump version and publish when ready.

Also, there seems to be unrelated changes. Do you know what those are from?

@po3na4skld
Copy link
Contributor Author

LGTM! Feel free to bump version and publish when ready.

Also, there seems to be unrelated changes. Do you know what those are from?

What changes?

@po3na4skld
version bump
a3c70cd
@po3na4skld
Merge remote-tracking branch 'origin/2920-Allow-configuring-Facebook-…
6ce6871 
…Marketing-insights-buffer-days' into 2920-Allow-configuring-Facebook-Marketing-insights-buffer-days
@po3na4skld
Copy link
Contributor Author

po3na4skld commented May 20, 2021 •
edited by github-actions bot
Loading

/publish connector=connectors/source-facebook-marketing

🕑 connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/859356280
✅ connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/859356280

@po3na4skld
Copy link
Contributor Author

po3na4skld commented May 20, 2021 •
edited by github-actions bot
Loading

/test connector=source-facebook-marketing

🕑 source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/859444398
✅ source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/859444398

@po3na4skld po3na4skld merged commit 6c96b81 into master May 20, 2021
@po3na4skld po3na4skld deleted the 2920-Allow-configuring-Facebook-Marketing-insights-buffer-days branch May 20, 2021 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sherifnada sherifnada sherifnada approved these changes

@yevhenii-ldv yevhenii-ldv Awaiting requested review from yevhenii-ldv

Assignees
No one assigned
Labels
connectors/source/facebook-marketing connectors/sources-api
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow configuring Facebook Marketing insights buffer days
3 participants
@po3na4skld @sherifnada @karinakuz