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 lookback window to insights streams #12402

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
d277560
WIP: add lookback window to insgiths streams
vladimir-remar Apr 27, 2022
03d5a72
update: docs in
vladimir-remar Apr 27, 2022
84efdab
update: add insights_lookback_window in custom insights
vladimir-remar May 10, 2022
c6d00d7
solve conflicts
vladimir-remar May 10, 2022
50a3a9d
Merge branch 'master' into source-facebook-marketing-add-insights-loo…
vladimir-remar May 19, 2022
d30b66d
update connector version in dockerfile and update facebook-marketing.md
vladimir-remar May 19, 2022
e12deff
fix conflicts
vladimir-remar May 20, 2022
71f326a
formatting using blackFormat
vladimir-remar May 20, 2022
b1481da
add minimun value to insights_lookback_window field and replace exclu…
vladimir-remar May 20, 2022
14b3007
fix unit tests: test_base_insight_streams
vladimir-remar May 20, 2022
439d527
fix order on specs
vladimir-remar May 23, 2022
a0d13f8
update integration spec file
vladimir-remar May 23, 2022
384682b
Merge branch 'master' into source-facebook-marketing-add-insights-loo…
alafanechere May 23, 2022
d549143
update test spec.json
alafanechere May 23, 2022
c86417f
update test spec.json
alafanechere May 23, 2022
3f67e25
fix conflicts
vladimir-remar May 24, 2022
fb66ecb
Merge branch 'master' into source-facebook-marketing-add-insights-loo…
vladimir-remar May 24, 2022
246c2dd
update refresh date
vladimir-remar May 24, 2022
b39262a
update test_base_insight_streams
vladimir-remar May 24, 2022
a6577ed
remove round brackets from insights_lookback_window description
vladimir-remar May 25, 2022
4524fc4
remove monkeypatch for AdsInsights in test_incremental_lookback_perio…
vladimir-remar May 25, 2022
fb99be7
Merge branch 'master' into source-facebook-marketing-add-insights-loo…
vladimir-remar May 30, 2022
fe1b30a
update connector version in Dockerfile
vladimir-remar May 30, 2022
b7d7973
fix typo in changelog
alafanechere May 30, 2022
944783d
Merge branch 'master' into source-facebook-marketing-add-insights-loo…
alafanechere May 31, 2022
7483176
auto-bump connector version
octavia-squidington-iii May 31, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def streams(self, config: Mapping[str, Any]) -> List[Type[Stream]]:
api=api,
start_date=config.start_date,
end_date=config.end_date,
insights_lookback_window=config.insights_lookback_window
)
streams = [
AdAccount(api=api),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,3 +147,13 @@ class Config:
"A list which contains insights entries, each entry must have a name and can contains fields, breakdowns or action_breakdowns)"
),
)

insights_lookback_window: Optional[PositiveInt] = Field(
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for your patience I will work on this ASAP.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

insights_lookback_window: Optional[PositiveInt] = Field(

title="Insights Lookback Window",
order=7,
description=(
"The attribution window"
),
exclusiveMaximum=28,
default=28,
)
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ class AdsInsights(FBMarketingIncrementalStream):
# HTTP response.
# https://developers.facebook.com/docs/marketing-api/reference/ad-account/insights/#overview
INSIGHTS_RETENTION_PERIOD = pendulum.duration(months=37)
# Facebook freezes insight data 28 days after it was generated, which means that all data
# from the past 28 days may have changed since we last emitted it, so we retrieve it again.
INSIGHTS_LOOKBACK_PERIOD = pendulum.duration(days=28)

action_breakdowns = ALL_ACTION_BREAKDOWNS
level = "ad"
Expand All @@ -64,6 +61,7 @@ def __init__(
breakdowns: List[str] = None,
action_breakdowns: List[str] = None,
time_increment: Optional[int] = None,
insights_lookback_window: int = None,
**kwargs,
):
super().__init__(**kwargs)
Expand All @@ -74,6 +72,7 @@ def __init__(
self.breakdowns = breakdowns or self.breakdowns
self.time_increment = time_increment or self.time_increment
self._new_class_name = name
self._insights_lookback_window = insights_lookback_window

# state
self._cursor_value: Optional[pendulum.Date] = None # latest period that was read
Expand All @@ -91,6 +90,16 @@ def primary_key(self) -> Optional[Union[str, List[str], List[List[str]]]]:
"""Build complex PK based on slices and breakdowns"""
return ["date_start", "account_id", "ad_id"] + self.breakdowns

@property
def insights_lookback_period(self):
"""
Facebook freezes insight data 28 days after it was generated, which means that all data
from the past 28 days may have changed since we last emitted it, so we retrieve it again.
But in some cases users my have define their own lookback window, thats
why the value for `insights_lookback_window` is set throught config.
"""
return pendulum.duration(days=self._insights_lookback_window)

def list_objects(self, params: Mapping[str, Any]) -> Iterable:
"""Because insights has very different read_records we don't need this method anymore"""

Expand Down Expand Up @@ -217,13 +226,12 @@ def _get_start_date(self) -> pendulum.Date:
"""
today = pendulum.today().date()
oldest_date = today - self.INSIGHTS_RETENTION_PERIOD
refresh_date = today - self.INSIGHTS_LOOKBACK_PERIOD

refresh_date = today - self.insights_lookback_period
if self._cursor_value:
start_date = self._cursor_value + pendulum.duration(days=self.time_increment)
if start_date > refresh_date:
logger.info(
f"The cursor value within refresh period ({self.INSIGHTS_LOOKBACK_PERIOD}), start sync from {refresh_date} instead."
f"The cursor value within refresh period ({self.insights_lookback_period}), start sync from {refresh_date} instead."
)
start_date = min(start_date, refresh_date)

Expand All @@ -234,7 +242,6 @@ def _get_start_date(self) -> pendulum.Date:
start_date = self._start_date
if start_date < oldest_date:
logger.warning(f"Loading insights older then {self.INSIGHTS_RETENTION_PERIOD} is not possible. Start sync from {oldest_date}.")

return max(oldest_date, start_date)

def request_params(self, **kwargs) -> MutableMapping[str, Any]:
Expand Down