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

TDL-9728: Stream ads_insights_age_gender has unexpected datatype for replication key field date_start #172

Merged
merged 4 commits into from
Nov 19, 2021
+39 −10
Merged

TDL-9728: Stream ads_insights_age_gender has unexpected datatype for replication key field date_start #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e5fab14
added format as date-time in schema file
Oct 13, 2021
c9373ec
added code coverage
namrata270998 Oct 21, 2021
527953a
added check for date format in the bookmark test
Nov 17, 2021
cbde97f
added the check for first sync messages
Nov 17, 2021
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
8 changes: 7 additions & 1 deletion .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,13 @@ jobs:
name: 'Run Unit Tests'
command: |
source /usr/local/share/virtualenvs/tap-facebook/bin/activate
nosetests tests/unittests
pip install nose coverage
nosetests --with-coverage --cover-erase --cover-package=tap_facebook --cover-html-dir=htmlcov tests/unittests
coverage html
- store_test_results:
path: test_output/report.xml
- store_artifacts:
path: htmlcov
- slack/notify-on-failure:
only_for_branches: master
run_integration_tests:
Expand Down
6 changes: 4 additions & 2 deletions tap_facebook/schemas/ads_insights_age_and_gender.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
"type": [
"null",
"string"
]
],
"format": "date-time"
},
"ad_id": {
"type": [
Expand Down Expand Up @@ -283,7 +284,8 @@
"type": [
"null",
"string"
]
],
"format": "date-time"
},
"objective": {
"type": [
Expand Down
6 changes: 4 additions & 2 deletions tap_facebook/schemas/ads_insights_hourly_advertiser.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,15 @@
"type": [
"null",
"string"
]
],
"format": "date-time"
},
"date_stop": {
"type": [
"null",
"string"
]
],
"format": "date-time"
},
"engagement_rate_ranking": {
"type": [
Expand Down
29 changes: 24 additions & 5 deletions tests/test_facebook_bookmarks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,16 @@ def calculated_states_by_stream(self, current_state):

return stream_to_calculated_state

# function for verifying the date format
def is_expected_date_format(self, date):
try:
# parse date
datetime.datetime.strptime(date, "%Y-%m-%dT%H:%M:%S.%fZ")
except ValueError:
# return False if date is in not expected format
return False
# return True in case of no error
return True

def test_run(self):
expected_streams = self.expected_streams()
Expand Down Expand Up @@ -193,14 +203,16 @@ def bookmarks_test(self, expected_streams):


for record in second_sync_messages:
# for "ads_insights_age_and_gender" and "ads_insights_hourly_advertiser"
# verify that the "date_start" and "date_stop" is in expected format
if stream in ["ads_insights_age_and_gender", "ads_insights_hourly_advertiser"]:
date_start = record.get("date_start")
self.assertTrue(self.is_expected_date_format(date_start))
date_stop = record.get("date_stop")
self.assertTrue(self.is_expected_date_format(date_stop))

Copy link

Choose a reason for hiding this comment

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

@harshpatel4crest Can you add a check in the integration test case to check the format of the date_start and date_stop. You can verify the format for only one record.. That would be enough.

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 the check for the date.

# Verify the second sync records respect the previous (simulated) bookmark value
replication_key_value = record.get(replication_key)
if stream in {'ads_insights_age_and_gender', 'ads_insights_hourly_advertiser'}: # BUG | https://stitchdata.atlassian.net/browse/SRCE-4873
replication_key_value = datetime.datetime.strftime(
dateutil.parser.parse(replication_key_value),
self.BOOKMARK_COMPARISON_FORMAT
)
self.assertGreaterEqual(replication_key_value, simulated_bookmark_minus_lookback,
msg="Second sync records do not repect the previous bookmark.")

Expand All @@ -211,6 +223,13 @@ def bookmarks_test(self, expected_streams):
)

for record in first_sync_messages:
# for "ads_insights_age_and_gender" and "ads_insights_hourly_advertiser"
# verify that the "date_start" and "date_stop" is in expected format
if stream in ["ads_insights_age_and_gender", "ads_insights_hourly_advertiser"]:
date_start = record.get("date_start")
self.assertTrue(self.is_expected_date_format(date_start))
date_stop = record.get("date_stop")
self.assertTrue(self.is_expected_date_format(date_stop))

# Verify the first sync bookmark value is the max replication key value for a given stream
replication_key_value = record.get(replication_key)
Expand Down