Source FB Marketing: add incremental support #1699

eugene-kulak · 2021-01-18T22:57:34Z

closes #1682

What

Describe what the change is solving
It helps to add screenshots if it affects the frontend.

How

Describe the solution

Pre-merge Checklist

Run integration tests
Publish Docker images

Tested scenarios

sync each stream with incremental/full method
sync stream with incremental method and state.json file

jrhizor · 2021-01-22T01:30:57Z

docs/integrations/sources/facebook-marketing.md

-* Gender & Age
-* Platform & Device
-* Region
+* Country - _comming soon_


Suggested change

* Country - _comming soon_

* Country - _coming soon_

for each

jrhizor · 2021-01-22T01:32:56Z

airbyte-integrations/connectors/source-facebook-marketing/source_facebook_marketing/spec.json

@@ -21,11 +21,6 @@
        "type": "string",
        "description": "The value of the access token generated. See the <a href=\"https://docs.airbyte.io/integrations/sources/facebook-marketing\">docs</a> for more information",
        "airbyte_secret": true
-      },
-      "include_deleted": {


Why is this removed?

it doesn't work yet, there is a ticket to add deleted records

jrhizor · 2021-01-22T03:35:24Z

...te-integrations/connectors/source-facebook-marketing/source_facebook_marketing/client/api.py

+
+
+#
+# class AdsInsightAPI(IncrementalStreamAPI):


Can you add a comment for why this is disabled in the code?

jrhizor · 2021-01-22T03:36:13Z

/test connector=source-facebook-marketing

🕑 source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/502771049
❌ source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/502771049

jrhizor

Generally looks good.

The integration test is failing. Once that's fixed please leave a comment and I'll take another look.

eugene-kulak · 2021-01-22T17:09:43Z

/test connector=source-facebook-marketing

🕑 source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/504210782
❌ source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/504210782

eugene-kulak · 2021-01-22T17:13:20Z

@jrhizor please continue

Test run finished after 81748 ms
[         2 containers found      ]
[         0 containers skipped    ]
[         2 containers started    ]
[         0 containers aborted    ]
[         2 containers successful ]
[         0 containers failed     ]
[         7 tests found           ]
[         0 tests skipped         ]
[         7 tests started         ]
[         0 tests aborted         ]
[         7 tests successful      ]
[         0 tests failed          ]

jrhizor · 2021-01-22T18:27:21Z

airbyte-integrations/connectors/source-facebook-marketing/unit_tests/test_api.py

+        api_mock = Mock()
+        api_mock.account.get_ad_creatives.return_value = [1, 2, 3]
+
+        api = AdCreativeAPI(api=api_mock)


Flake8 is failing to lint this due to:

[python] .venv/bin/python -m flake8 . --config /home/runner/work/airbyte/airbyte/tools/python/.flake8 | -- | -- | ./unit_tests/test_api.py:42:9: F841 local variable 'api' is assigned to but never used | | ./unit_tests/test_api.py:45:9: F821 undefined name 'api' | | ./unit_tests/test_api.py:48:9: F821 undefined name 'api'

eugene-kulak · 2021-01-22T19:31:45Z

@jrhizor I had to do force push because one commit from source-file PR was added in this PR by accident

eugene-kulak · 2021-01-22T20:46:08Z

/test connector=source-facebook-marketing

🕑 source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/504604817
❌ source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/504604817

eugene-kulak · 2021-01-22T20:59:39Z

/test connector=source-facebook-marketing

🕑 source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/504627662
❌ source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/504627662

eugene-kulak · 2021-01-22T21:16:26Z

/test connector=source-facebook-marketing

🕑 source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/504655813
❌ source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/504655813

eugene-kulak · 2021-01-22T22:42:39Z

/test connector=source-facebook-marketing

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

sherifnada · 2021-01-22T22:46:50Z

/test connector=source-facebook-marketing

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

eugene-kulak added 11 commits January 19, 2021 00:55

add incremental support

74c0d13

fix merge

c1febb2

fixme

9a9915a

fix build

ab2570d

format

b1a7e49

fix tests

54d174b

fix issues

0497f17

fix creatives API, extend configured_catalog.json

7e0cea1

fix missing fields in insights stream

897428b

fix incremental sync for insights

7616cde

update docs

c6cd99e

eugene-kulak changed the title ~~WIP: Source FB Marketing: add incremental support~~ Source FB Marketing: add incremental support Jan 19, 2021

eugene-kulak added 5 commits January 19, 2021 05:12

fix broken links

793844d

format

caf5f04

format, cut test catalog.

29795af

fix abc class, replace with runtime check

dd876d0

drop Ads from catalog, as there is only single ad

4d28969

eugene-kulak mentioned this pull request Jan 19, 2021

Broken links in FB docs #1682

Closed

split api and client

452ef90

eugene-kulak mentioned this pull request Jan 19, 2021

Source Facebook Marketing: populate test account with insight data #1709

Closed

eugene-kulak added 2 commits January 19, 2021 18:15

disable insights streams

2524a69

format

0aac2ae

eugene-kulak mentioned this pull request Jan 19, 2021

adopt connector best practices for FB Marketing source #1566

Closed

5 tasks

jrhizor self-requested a review January 22, 2021 01:24

jrhizor reviewed Jan 22, 2021

View reviewed changes

jrhizor suggested changes Jan 22, 2021

View reviewed changes

jrhizor reviewed Jan 22, 2021

View reviewed changes

eugene-kulak added 3 commits January 22, 2021 21:29

WIP FB tests

501d53f

address suggestions from @jrhizor

ad900ac

fix catalog

219b11a

eugene-kulak force-pushed the keu/source-facebook-marketing-incremental branch from ae4dd57 to 219b11a Compare January 22, 2021 19:30

eugene-kulak added 3 commits January 22, 2021 22:05

flake

b51e5d6

format

d380e32

format

3cf25e3

sherifnada linked an issue Jan 22, 2021 that may be closed by this pull request

Source Facebook Marketing: Add incremental update #1478

Closed

3 tasks

bump

8779067

sherifnada merged commit 9b09994 into master Jan 22, 2021

sherifnada deleted the keu/source-facebook-marketing-incremental branch January 22, 2021 23:34

karinakuz added connectors/sources-api connectors/source/facebook-marketing labels Jan 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Source FB Marketing: add incremental support #1699

Source FB Marketing: add incremental support #1699

eugene-kulak commented Jan 18, 2021 •

edited by cgardens

Loading

jrhizor Jan 22, 2021

jrhizor Jan 22, 2021

eugene-kulak Jan 22, 2021

jrhizor Jan 22, 2021

jrhizor commented Jan 22, 2021 •

edited by github-actions bot

Loading

jrhizor left a comment

eugene-kulak commented Jan 22, 2021 •

edited by github-actions bot

Loading

eugene-kulak commented Jan 22, 2021

jrhizor Jan 22, 2021

eugene-kulak commented Jan 22, 2021

eugene-kulak commented Jan 22, 2021 •

edited by github-actions bot

Loading

eugene-kulak commented Jan 22, 2021 •

edited by github-actions bot

Loading

eugene-kulak commented Jan 22, 2021 •

edited by github-actions bot

Loading

eugene-kulak commented Jan 22, 2021 •

edited by github-actions bot

Loading

sherifnada commented Jan 22, 2021 •

edited by github-actions bot

Loading

Source FB Marketing: add incremental support #1699

Source FB Marketing: add incremental support #1699

Conversation

eugene-kulak commented Jan 18, 2021 • edited by cgardens Loading

What

How

Pre-merge Checklist

Recommended reading order

Tested scenarios

jrhizor Jan 22, 2021

Choose a reason for hiding this comment

jrhizor Jan 22, 2021

Choose a reason for hiding this comment

eugene-kulak Jan 22, 2021

Choose a reason for hiding this comment

jrhizor Jan 22, 2021

Choose a reason for hiding this comment

jrhizor commented Jan 22, 2021 • edited by github-actions bot Loading

jrhizor left a comment

Choose a reason for hiding this comment

eugene-kulak commented Jan 22, 2021 • edited by github-actions bot Loading

eugene-kulak commented Jan 22, 2021

jrhizor Jan 22, 2021

Choose a reason for hiding this comment

eugene-kulak commented Jan 22, 2021

eugene-kulak commented Jan 22, 2021 • edited by github-actions bot Loading

eugene-kulak commented Jan 22, 2021 • edited by github-actions bot Loading

eugene-kulak commented Jan 22, 2021 • edited by github-actions bot Loading

eugene-kulak commented Jan 22, 2021 • edited by github-actions bot Loading

sherifnada commented Jan 22, 2021 • edited by github-actions bot Loading

eugene-kulak commented Jan 18, 2021 •

edited by cgardens

Loading

jrhizor commented Jan 22, 2021 •

edited by github-actions bot

Loading

eugene-kulak commented Jan 22, 2021 •

edited by github-actions bot

Loading

eugene-kulak commented Jan 22, 2021 •

edited by github-actions bot

Loading

eugene-kulak commented Jan 22, 2021 •

edited by github-actions bot

Loading

eugene-kulak commented Jan 22, 2021 •

edited by github-actions bot

Loading

eugene-kulak commented Jan 22, 2021 •

edited by github-actions bot

Loading

sherifnada commented Jan 22, 2021 •

edited by github-actions bot

Loading