Source Facebook Marketing: Add lookback window to insights streams by vladimir-remar · Pull Request #12402 · airbytehq/airbyte

Vladimir remar (vladimir-remar) · 2022-04-27T15:16:42Z

What

Bring back the possibility to set up an attribution window.
Let say the users will not use all the data retrieve from the insight stream and just use part of it (e.g., spend) in some cases
set a looback_window short than 28 days would help the users by reducing the sync time.

How

Set value for insights_lookback_window throught config.

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

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
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

Vladimir remar (vladimir-remar) · 2022-04-27T15:18:32Z

Sherif A. Nada (@sherifnada) I made this Draft related to #9805

M. Marx (marcosmarxm) · 2022-05-02T17:43:42Z

Vladimir remar (@vladimir-remar) this is ready to review?

M. Marx (marcosmarxm) · 2022-05-02T23:03:15Z

Serhii Lazebnyi (@lazebnyi) can you check the implementation Vladimir remar (@vladimir-remar) is doing for Facebook Marketing?

Sherif A. Nada (sherifnada) · 2022-05-02T23:30:51Z

        ),
    )
+
+    insights_lookback_window: Optional[PositiveInt] = Field(


we should make this default_insights_lookback_window and apply it only to the default insights synced by this connector, and separately we should allow configuring this on a per-insight level in the custom insights

thanks for your patience I will work on this ASAP.

Hey Vladimir remar (@vladimir-remar) I'm not sure you ended up implementing Sherif A. Nada (@sherifnada) suggestion. Is anything blocking you from going in the suggested direction?

Hello Augustin (@alafanechere), maybe I'm wrong, I just replicate the same approach in the custom insight part.

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

Line 80 in c6d00d7

insights_lookback_window: Optional[PositiveInt] = Field(

CLAassistant · 2022-05-05T13:09:19Z

All committers have signed the CLA.

Augustin (alafanechere) · 2022-05-16T19:41:31Z

        ),
    )
+
+    insights_lookback_window: Optional[PositiveInt] = Field(


Hey Vladimir remar (@vladimir-remar) I'm not sure you ended up implementing Sherif A. Nada (@sherifnada) suggestion. Is anything blocking you from going in the suggested direction?

Augustin (alafanechere)

Vladimir remar (@vladimir-remar) I think you're right, Sherif A. Nada (@sherifnada)'s concern is addressed, 👍 I'm requesting change as I think you need to update unit tests to cover your changes. This connector is in General Availability, meaning we are quite strict in terms of coverage. Could you please bump the connector version in the Dockerfile and update the changelog in facebook-marketing.md?

Augustin (alafanechere) · 2022-05-18T14:23:15Z

I would also need an additional review from Sherif A. Nada (@sherifnada) or Serhii Lazebnyi (@lazebnyi) 🙏

Augustin (alafanechere) · 2022-05-18T14:23:54Z

/test connector=connectors/source-facebook-marketing

🕑 connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2346152756
❌ connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2346152756
🐛 https://gradle.com/s/gghga2qcl74iw

…kback-window

Serhii Lazebnyi (lazebnyi) · 2022-05-19T12:33:58Z

Augustin (@alafanechere) I think Serhii Chvaliuk (@grubberr) can help with review. Already discussed with him about that. He working now on FB marketing.

Serhii Chvaliuk (grubberr) · 2022-05-20T04:44:51Z

Vladimir remar (@vladimir-remar) please also fix unit_tests

pytest unit_tests


Results (1.28s):
      82 passed
      16 failed

Serhii Chvaliuk (grubberr) · 2022-05-24T07:58:56Z

/test connector=connectors/source-facebook-marketing

🕑 connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2376340659
❌ connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2376340659
🐛 https://gradle.com/s/ke2czo2he57ws

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 FAILED unit_tests/test_base_insight_streams.py::TestBaseInsightsStream::test_stream_slices_no_state
	 FAILED unit_tests/test_base_insight_streams.py::TestBaseInsightsStream::test_stream_slices_no_state_close_to_now
	 FAILED unit_tests/test_base_insight_streams.py::TestBaseInsightsStream::test_stream_slices_with_state
	 FAILED unit_tests/test_base_insight_streams.py::TestBaseInsightsStream::test_stream_slices_with_state_close_to_now
	 FAILED unit_tests/test_base_insight_streams.py::TestBaseInsightsStream::test_stream_slices_with_state_and_slices
	 FAILED unit_tests/test_base_insight_streams.py::TestBaseInsightsStream::test_completed_slices_in_lookback_period
	 FAILED unit_tests/test_base_insight_streams.py::TestBaseInsightsStream::test_incremental_lookback_period_updated
	 �[31m========================= �[31m�[1m7 failed�[0m, �[32m93 passed�[0m�[31m in 1.47s�[0m�[31m =========================�[0m

Augustin (alafanechere) · 2022-05-24T08:41:08Z

Hey Serhii Chvaliuk (@grubberr) do you have an idea why tests are failing now after passing yesterday? Did you merge master in the branch?
Vladimir remar (@vladimir-remar) I will be off until next Monday, bare with me for the review delay. Serhii Chvaliuk (@grubberr) feel free to proceed to publish and merge once we get a review from Andy (@misteryeo) .

Vladimir remar (vladimir-remar) · 2022-05-24T08:48:14Z

Hey Serhii Chvaliuk (@grubberr) do you have an idea why tests are failing now after passing yesterday? Did you merge master in the branch? Vladimir remar (@vladimir-remar) I will be off until next Monday, bare with me for the review delay. Serhii Chvaliuk (@grubberr) feel free to proceed to publish and merge once we get a review from Andy (@misteryeo) .

Augustin (@alafanechere) Yes I did today after see the conflict.

Serhii Chvaliuk (grubberr) · 2022-05-24T11:22:28Z

Vladimir remar (@vladimir-remar) I think unit_tests failed because the master branch changed and you need to synchronise your code base with the latest updates in the master

…kback-window

Serhii Chvaliuk (grubberr) · 2022-05-24T15:37:43Z

/test connector=connectors/source-facebook-marketing

🕑 connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2378940847
✅ connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2378940847
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        77      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/tests/test_incremental.py       121     25    79%
source_acceptance_test/utils/common.py                  80     17    79%
source_acceptance_test/tests/test_core.py              294    106    64%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
------------------------------------------------------------------------
TOTAL                                                  960    246    74%
Name                                                        Stmts   Miss  Cover
-------------------------------------------------------------------------------
source_facebook_marketing/streams/__init__.py                   2      0   100%
source_facebook_marketing/spec.py                              36      0   100%
source_facebook_marketing/__init__.py                           2      0   100%
source_facebook_marketing/api.py                               96     12    88%
source_facebook_marketing/streams/base_streams.py             127     27    79%
source_facebook_marketing/streams/common.py                    41     13    68%
source_facebook_marketing/streams/streams.py                   97     32    67%
source_facebook_marketing/source.py                            39     16    59%
source_facebook_marketing/streams/base_insight_streams.py     147     69    53%
source_facebook_marketing/streams/async_job.py                210    134    36%
source_facebook_marketing/streams/async_job_manager.py         78     60    23%
-------------------------------------------------------------------------------
TOTAL                                                         875    363    59%
Name                                                        Stmts   Miss  Cover
-------------------------------------------------------------------------------
source_facebook_marketing/streams/async_job.py                210      0   100%
source_facebook_marketing/streams/__init__.py                   2      0   100%
source_facebook_marketing/spec.py                              36      0   100%
source_facebook_marketing/__init__.py                           2      0   100%
source_facebook_marketing/streams/common.py                    41      1    98%
source_facebook_marketing/source.py                            39      1    97%
source_facebook_marketing/streams/async_job_manager.py         78      3    96%
source_facebook_marketing/streams/base_insight_streams.py     147      6    96%
source_facebook_marketing/api.py                               96      9    91%
source_facebook_marketing/streams/streams.py                   97     22    77%
source_facebook_marketing/streams/base_streams.py             127     30    76%
-------------------------------------------------------------------------------
TOTAL                                                         875     72    92%

Build Passed

Test summary info:

All Passed

…d_updated and test_completed_slices_in_lookback_period

Serhii Chvaliuk (grubberr) · 2022-05-25T08:38:52Z

/test connector=connectors/source-facebook-marketing

🕑 connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2383206314
✅ connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2383206314
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        77      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/tests/test_incremental.py       121     25    79%
source_acceptance_test/utils/common.py                  80     17    79%
source_acceptance_test/tests/test_core.py              294    106    64%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
------------------------------------------------------------------------
TOTAL                                                  960    246    74%
Name                                                        Stmts   Miss  Cover
-------------------------------------------------------------------------------
source_facebook_marketing/streams/__init__.py                   2      0   100%
source_facebook_marketing/spec.py                              36      0   100%
source_facebook_marketing/__init__.py                           2      0   100%
source_facebook_marketing/api.py                               96     12    88%
source_facebook_marketing/streams/base_streams.py             127     27    79%
source_facebook_marketing/streams/common.py                    41     13    68%
source_facebook_marketing/streams/streams.py                   97     32    67%
source_facebook_marketing/source.py                            39     16    59%
source_facebook_marketing/streams/base_insight_streams.py     147     69    53%
source_facebook_marketing/streams/async_job.py                210    134    36%
source_facebook_marketing/streams/async_job_manager.py         78     60    23%
-------------------------------------------------------------------------------
TOTAL                                                         875    363    59%
Name                                                        Stmts   Miss  Cover
-------------------------------------------------------------------------------
source_facebook_marketing/streams/async_job.py                210      0   100%
source_facebook_marketing/streams/__init__.py                   2      0   100%
source_facebook_marketing/spec.py                              36      0   100%
source_facebook_marketing/__init__.py                           2      0   100%
source_facebook_marketing/streams/common.py                    41      1    98%
source_facebook_marketing/source.py                            39      1    97%
source_facebook_marketing/streams/async_job_manager.py         78      3    96%
source_facebook_marketing/streams/base_insight_streams.py     147      6    96%
source_facebook_marketing/api.py                               96      9    91%
source_facebook_marketing/streams/streams.py                   97     22    77%
source_facebook_marketing/streams/base_streams.py             127     30    76%
-------------------------------------------------------------------------------
TOTAL                                                         875     72    92%

Build Passed

Test summary info:

All Passed

…kback-window

Augustin (alafanechere)

Thank you Serhii Chvaliuk (@grubberr) for the review and Vladimir remar (@vladimir-remar) for the changes! I'm now waiting for Andy (@misteryeo) approval to publish and merge.

Augustin (alafanechere) · 2022-05-31T06:49:49Z

/publish connector=connectors/source-facebook-marketing

🕑 connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2413164173
❌ Failed to publish connectors/source-facebook-marketing
❌ Couldn't auto-bump version for connectors/source-facebook-marketing
🕑 connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2413164173
🚀 Successfully published connectors/source-facebook-marketing
🚀 Auto-bumped version for connectors/source-facebook-marketing
✅ connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2413164173

…kback-window

Augustin (alafanechere) · 2022-05-31T07:37:31Z

Thank you Vladimir remar (@vladimir-remar) for the contribution! The new connector version is now published and I'm going to merge this PR.

…irbytehq#12402)

WIP: add lookback window to insgiths streams

d277560

github-actions Bot added the area/connectors Connector related issues label Apr 27, 2022

Octavia Squidington III (octavia-squidington-iii) added the community label Apr 27, 2022

update: docs in

03d5a72

Vladimir remar (vladimir-remar) changed the title ~~Add lookback window to insgiths streams~~ Source Facebook Marketing: Add lookback window to insgiths streams Apr 28, 2022

M. Marx (marcosmarxm) requested a review from Serhii Lazebnyi (lazebnyi) April 28, 2022 16:03

Sherif A. Nada (sherifnada) reviewed May 2, 2022

View reviewed changes

update: add insights_lookback_window in custom insights

84efdab

Vladimir remar (vladimir-remar) marked this pull request as ready for review May 10, 2022 11:56

solve conflicts

c6d00d7

Vladimir remar (vladimir-remar) requested a review from Sherif A. Nada (sherifnada) May 10, 2022 12:01

Augustin (alafanechere) self-assigned this May 16, 2022

Augustin (alafanechere) suggested changes May 16, 2022

View reviewed changes

Augustin (alafanechere) suggested changes May 18, 2022

View reviewed changes

Augustin (alafanechere) added connectors/source/facebook-marketing team/connectors-python labels May 18, 2022

Vladimir remar (vladimir-remar) added 2 commits May 19, 2022 09:08

Merge branch 'master' into source-facebook-marketing-add-insights-loo…

50a3a9d

…kback-window

update connector version in dockerfile and update facebook-marketing.md

d30b66d

github-actions Bot added the area/documentation Improvements or additions to documentation label May 19, 2022

Serhii Lazebnyi (lazebnyi) requested review from Serhii Chvaliuk (grubberr) and removed request for Serhii Lazebnyi (lazebnyi) May 19, 2022 12:31

Serhii Chvaliuk (grubberr) suggested changes May 19, 2022

View reviewed changes

Comment thread docs/integrations/sources/facebook-marketing.md Outdated

Comment thread airbyte-integrations/connectors/source-facebook-marketing/source_facebook_marketing/spec.py Outdated

Comment thread airbyte-integrations/connectors/source-facebook-marketing/source_facebook_marketing/spec.py

Merge branch 'master' into source-facebook-marketing-add-insights-loo…

fb66ecb

…kback-window

Vladimir remar (vladimir-remar) closed this May 24, 2022

Vladimir remar (vladimir-remar) reopened this May 24, 2022

Vladimir remar (vladimir-remar) added 2 commits May 24, 2022 15:42

update refresh date

246c2dd

update test_base_insight_streams

b39262a

Serhii Chvaliuk (grubberr) suggested changes May 25, 2022

View reviewed changes

Vladimir remar (vladimir-remar) added 2 commits May 25, 2022 09:36

remove round brackets from insights_lookback_window description

a6577ed

remove monkeypatch for AdsInsights in test_incremental_lookback_perio…

4524fc4

…d_updated and test_completed_slices_in_lookback_period

Serhii Chvaliuk (grubberr) approved these changes May 25, 2022

View reviewed changes

Vladimir remar (vladimir-remar) added 2 commits May 30, 2022 08:58

Merge branch 'master' into source-facebook-marketing-add-insights-loo…

fb99be7

…kback-window

update connector version in Dockerfile

fe1b30a

Vladimir remar (vladimir-remar) requested a review from Augustin (alafanechere) May 30, 2022 07:01

fix typo in changelog

b7d7973

Augustin (alafanechere) approved these changes May 30, 2022

View reviewed changes

Andy (misteryeo) approved these changes May 31, 2022

View reviewed changes

Augustin (alafanechere) and others added 2 commits May 31, 2022 08:55

Merge branch 'master' into source-facebook-marketing-add-insights-loo…

944783d

…kback-window

auto-bump connector version

7483176

Augustin (alafanechere) merged commit 76032e6 into airbytehq:master May 31, 2022

Octavia Squidington III (octavia-squidington-iii) mentioned this pull request May 31, 2022

Bump Airbyte version from 0.39.5-alpha to 0.39.6-alpha #13350

Merged

Jordan Scott (jscottpolevault) pushed a commit to jscottpolevault/airbyte that referenced this pull request Jun 1, 2022

Source Facebook Marketing: Add lookback window to insights streams (a…

045fc96

…irbytehq#12402)

Uh oh!

Conversation

Vladimir remar (vladimir-remar) commented Apr 27, 2022

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Tests

Uh oh!

Vladimir remar (vladimir-remar) commented Apr 27, 2022

Uh oh!

M. Marx (marcosmarxm) commented May 2, 2022

Uh oh!

M. Marx (marcosmarxm) commented May 2, 2022

Uh oh!

Sherif A. Nada (sherifnada) May 2, 2022

Choose a reason for hiding this comment

Uh oh!

Vladimir remar (vladimir-remar) May 5, 2022

Choose a reason for hiding this comment

Uh oh!

Augustin (alafanechere) May 16, 2022

Choose a reason for hiding this comment

Uh oh!

Vladimir remar (vladimir-remar) May 17, 2022

Choose a reason for hiding this comment

Uh oh!

CLAassistant commented May 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Augustin (alafanechere) May 16, 2022

Choose a reason for hiding this comment

Uh oh!

Augustin (alafanechere) left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Augustin (alafanechere) commented May 18, 2022

Uh oh!

Augustin (alafanechere) commented May 18, 2022 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Serhii Lazebnyi (lazebnyi) commented May 19, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Serhii Chvaliuk (grubberr) commented May 20, 2022

Uh oh!

Serhii Chvaliuk (grubberr) commented May 24, 2022 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Failed

Uh oh!

Augustin (alafanechere) commented May 24, 2022

Uh oh!

Vladimir remar (vladimir-remar) commented May 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Serhii Chvaliuk (grubberr) commented May 24, 2022

Uh oh!

Serhii Chvaliuk (grubberr) commented May 24, 2022 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Passed

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Serhii Chvaliuk (grubberr) commented May 25, 2022 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Passed

Uh oh!

Augustin (alafanechere) left a comment

Choose a reason for hiding this comment

Uh oh!

Augustin (alafanechere) commented May 31, 2022 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented May 5, 2022 •

edited

Loading

Augustin (alafanechere) left a comment •

edited

Loading

Augustin (alafanechere) commented May 18, 2022 •

edited by github-actions Bot

Loading

Serhii Chvaliuk (grubberr) commented May 24, 2022 •

edited by github-actions Bot

Loading

Vladimir remar (vladimir-remar) commented May 24, 2022 •

edited

Loading

Serhii Chvaliuk (grubberr) commented May 24, 2022 •

edited by github-actions Bot

Loading

Serhii Chvaliuk (grubberr) commented May 25, 2022 •

edited by github-actions Bot

Loading

Augustin (alafanechere) commented May 31, 2022 •

edited by github-actions Bot

Loading