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

TDL-18879: Clear offset when max_skip error is encountered and return 0 if skip is greater than 250k in the current state. #29

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
15 changes: 9 additions & 6 deletions tap_closeio/streams.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@

LOGGER = singer.get_logger()

# default date date window size in days
DATE_WINDOW_SIZE = 5

PATHS = {
IDS.CUSTOM_FIELDS: "/custom_fields/lead/",
IDS.LEADS: "/lead/",
Expand Down Expand Up @@ -168,15 +171,15 @@ def sync_activities(ctx):

try:
# get date window from config
date_window = int(ctx.config.get("date_window", 15))
# if date_window is 0, '0' or None, then set default window size of 15 days
date_window = int(ctx.config.get("date_window", DATE_WINDOW_SIZE))
# if date_window is 0, '0' or None, then set default window size of 5 days

Choose a reason for hiding this comment

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

Suggested change
# if date_window is 0, '0' or None, then set default window size of 5 days
# if date_window is 0, '0' or None, then set the default window size to DATE_WINDOW_SIZE(5 days)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

if not date_window:
LOGGER.warning("Invalid value of date window is passed: \'{}\', using default window size of 15 days.".format(ctx.config.get("date_window")))
date_window = 15
LOGGER.warning("Invalid value of date window is passed: \'{}\', using default window size of 5 days.".format(ctx.config.get("date_window")))
Copy link

@dbshah1212 dbshah1212 May 3, 2022

Choose a reason for hiding this comment

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

Suggested change
LOGGER.warning("Invalid value of date window is passed: \'{}\', using default window size of 5 days.".format(ctx.config.get("date_window")))
LOGGER.warning("Invalid value of date window is passed: \'{}\', using default window size of {} days.".format(ctx.config.get("date_window")),DATE_WINDOW_SIZE)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

date_window = DATE_WINDOW_SIZE
except ValueError:
LOGGER.warning("Invalid value of date window is passed: \'{}\', using default window size of 15 days.".format(ctx.config.get("date_window")))
LOGGER.warning("Invalid value of date window is passed: \'{}\', using default window size of 5 days.".format(ctx.config.get("date_window")))
Copy link

@dbshah1212 dbshah1212 May 3, 2022

Choose a reason for hiding this comment

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

Suggested change
LOGGER.warning("Invalid value of date window is passed: \'{}\', using default window size of 5 days.".format(ctx.config.get("date_window")))
LOGGER.warning("Invalid value of date window is passed: \'{}\', using default window size of {} days.".format(ctx.config.get("date_window")), DATE_WINDOW_SIZE)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

# In case of empty string(''), use default window
date_window = 15
date_window = DATE_WINDOW_SIZE

LOGGER.info("Using offset seconds {}".format(offset_secs))
start_date -= timedelta(seconds=offset_secs)
Expand Down
30 changes: 17 additions & 13 deletions tests/unittests/test_activity_stream_date_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ class TestActivityStreamDateWindow(unittest.TestCase):

def test_activity_stream_default_date_window(self, mocked_paginated_sync):
"""
Test case to verify we are calling Activity API in 15 days window (default) when no "date_window" is passed in the config
Test case to verify we are calling Activity API in 5 days window (default) when no "date_window" is passed in the config
"""
# now date
now_date = datetime.now()
config = {
"start_date": (now_date - timedelta(days=40)).strftime("%Y-%m-%d/"), # set date 40 days later than now
"start_date": (now_date - timedelta(days=40)).strftime("%Y-%m-%d"), # set date 40 days later than now
"api_key": "test_API_key"
}
state = {}
Expand All @@ -25,13 +25,14 @@ def test_activity_stream_default_date_window(self, mocked_paginated_sync):
# function call
sync_activities(ctx)

# verify we called 'paginated_sync' 3 times as the start date is 40 days later and default date window is 15 days
self.assertEqual(mocked_paginated_sync.call_count, 3)
# verify we called 'paginated_sync' 3 times as the start date is 40 days later and default date window is 5 days
# as the default lookback window of 1 day is used, as a result, there will be additional 1 API call
self.assertEqual(mocked_paginated_sync.call_count, 9)

@mock.patch("tap_closeio.streams.LOGGER.warning")
def test_activity_stream_empty_string_date_window(self, mocked_logger_warning, mocked_paginated_sync):
"""
Test case to verify we are calling Activity API in 15 days window (default) when empty string "date_window" is passed in the config
Test case to verify we are calling Activity API in 5 days window (default) when empty string "date_window" is passed in the config
"""
# now date
now_date = datetime.now()
Expand All @@ -49,14 +50,15 @@ def test_activity_stream_empty_string_date_window(self, mocked_logger_warning, m
sync_activities(ctx)

# verify we called 'paginated_sync' 14 times as the start date is 40 days later and we have set default date window
self.assertEqual(mocked_paginated_sync.call_count, 3)
# as the default lookback window of 1 day is used, as a result, there will be additional 1 API call
self.assertEqual(mocked_paginated_sync.call_count, 9)
# verify warning is raised for invalid date window
mocked_logger_warning.assert_called_with("Invalid value of date window is passed: '', using default window size of 15 days.")
mocked_logger_warning.assert_called_with("Invalid value of date window is passed: '', using default window size of 5 days.")

@mock.patch("tap_closeio.streams.LOGGER.warning")
def test_activity_stream_0_date_window(self, mocked_logger_warning, mocked_paginated_sync):
"""
Test case to verify we are calling Activity API in 15 days window (default) when int 0 "date_window" is passed in the config
Test case to verify we are calling Activity API in 5 days window (default) when int 0 "date_window" is passed in the config
"""
# now date
now_date = datetime.now()
Expand All @@ -74,14 +76,15 @@ def test_activity_stream_0_date_window(self, mocked_logger_warning, mocked_pagin
sync_activities(ctx)

# verify we called 'paginated_sync' 14 times as the start date is 40 days later and we have set default date window
self.assertEqual(mocked_paginated_sync.call_count, 3)
# as the default lookback window of 1 day is used, as a result, there will be additional 1 API call
self.assertEqual(mocked_paginated_sync.call_count, 9)
# verify warning is raised for invalid date window
mocked_logger_warning.assert_called_with("Invalid value of date window is passed: '0', using default window size of 15 days.")
mocked_logger_warning.assert_called_with("Invalid value of date window is passed: '0', using default window size of 5 days.")

@mock.patch("tap_closeio.streams.LOGGER.warning")
def test_activity_stream_string_0_date_window(self, mocked_logger_warning, mocked_paginated_sync):
"""
Test case to verify we are calling Activity API in 15 days window (default) when string 0 "date_window" is passed in the config
Test case to verify we are calling Activity API in 5 days window (default) when string 0 "date_window" is passed in the config
"""
# now date
now_date = datetime.now()
Expand All @@ -99,9 +102,10 @@ def test_activity_stream_string_0_date_window(self, mocked_logger_warning, mocke
sync_activities(ctx)

# verify we called 'paginated_sync' 14 times as the start date is 40 days later and we have set default date window
self.assertEqual(mocked_paginated_sync.call_count, 3)
# as the default lookback window of 1 day is used, as a result, there will be additional 1 API call
self.assertEqual(mocked_paginated_sync.call_count, 9)
# verify warning is raised for invalid date window
mocked_logger_warning.assert_called_with("Invalid value of date window is passed: '0', using default window size of 15 days.")
mocked_logger_warning.assert_called_with("Invalid value of date window is passed: '0', using default window size of 5 days.")

def test_activity_stream_configurable_date_window(self, mocked_paginated_sync):
"""
Expand Down