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-17610: Multiple apps support and data fetching #94

Merged
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
521991b
Implement configurable app_ids and wirte test case for them
karanpanchal-crest Apr 6, 2022
07ce0bb
correct message in unit tets
karanpanchal-crest Apr 6, 2022
1685dd4
change README.md file for multiapp selection
karanpanchal-crest Apr 6, 2022
82a1f7d
added logger and exception message in app_ids fetching logic
karanpanchal-crest Apr 7, 2022
8f709c6
Add test cases and correct exception and logger message
karanpanchal-crest Apr 7, 2022
f043603
chnage logic for app_ids configuration
karanpanchal-crest Apr 8, 2022
aea8de0
correct pylint error
karanpanchal-crest Apr 8, 2022
df1bfc9
correct Trailing whitespace error
karanpanchal-crest Apr 8, 2022
83bd358
Add asserts in unittest
karanpanchal-crest Apr 18, 2022
8e8d156
placed invalid app_ids exception inside the main function
karanpanchal-crest Apr 19, 2022
f70f51e
add app_ids exception to discovery and sync mode
karanpanchal-crest Apr 19, 2022
93f061c
Add Integration test for the app_ids
karanpanchal-crest Apr 27, 2022
a935175
add integration test comments for app_ids
karanpanchal-crest Apr 27, 2022
3cf8ef2
updated filter function
somethingmorerelevant Mar 21, 2023
b42f285
Merge branch 'master' into TDL-17610-multiple-apps-support-and-data-f…
somethingmorerelevant Mar 23, 2023
c5ae630
updatd filter statement
somethingmorerelevant Mar 23, 2023
470cda8
Merge branch 'master' into TDL-17610-multiple-apps-support-and-data-f…
somethingmorerelevant Apr 3, 2023
4faca9f
Merge branch 'master' into TDL-17610-multiple-apps-support-and-data-f…
somethingmorerelevant Apr 10, 2023
a80271b
fixed broken UT
somethingmorerelevant Apr 10, 2023
e701b24
Merge branch 'master' into TDL-17610-multiple-apps-support-and-data-f…
somethingmorerelevant Apr 10, 2023
ab08c89
fix multi app-ids support to all streams
Apr 11, 2023
0c0a1d4
- fix integration test reviews
RushiT0122 Apr 12, 2023
f82d5e6
updated readme
somethingmorerelevant Jul 24, 2023
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
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ Interrupted syncs for Event type stream are resumed via a bookmark placed during
- `period` (string, `ABCdef123`): `dayRange` or `hourRange`
- `lookback_window` (integer): 10 (For event objects. Default: 0)
- `request_timeout` (integer): 300 (For passing timeout to the request. Default: 300)
- `app_ids` (string, `abcd123, Abcd567`): (comma seperated appIDs. If this parameter is not provided, then the data will be collected from all the apps.)

```json
{
Expand All @@ -254,7 +255,8 @@ Interrupted syncs for Event type stream are resumed via a bookmark placed during
"period": "dayRange",
"lookback_window": 10,
"request_timeout": 300,
"include_anonymous_visitors": "true"
"include_anonymous_visitors": "true",
"app_ids": "abcd123, Abcd567"
}
```

Expand Down
21 changes: 21 additions & 0 deletions tap_pendo/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,34 @@ def sync(config, state, catalog):
singer.write_state(state)
LOGGER.info("Finished sync")

# To check that given number is numeric or not
def isInteger(n):
try:
int(n)
return False
except Exception:
return True

@utils.handle_top_exception(LOGGER)
def main():

# Parse command line arguments
args = utils.parse_args(REQUIRED_CONFIG_KEYS)

# If app_ids is not in configure file then get all apps data
app_ids = args.config.get("app_ids", "expandAppIds(\"*\")").replace(" ", "") or "expandAppIds(\"*\")"
# If app_ids is given in configure file then check all app_ids are numeric or not
invalid_app_ids = []
if app_ids != "expandAppIds(\"*\")":
app_ids = app_ids.split(",")
invalid_app_ids = list(filter(isInteger, app_ids))

args.config["app_ids"] = app_ids

if args.discover:
# If non numeric app_ids are given in configure file then raise exception
dsprayberry marked this conversation as resolved.
Show resolved Hide resolved
if len(invalid_app_ids) > 0:
raise Exception('Invalid appIDs provided during the configuration:{}'.format(invalid_app_ids)) from None
do_discover(args.config)
elif args.catalog:
state = args.state
Expand Down
3 changes: 3 additions & 0 deletions tap_pendo/schemas/features.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
{
"type": ["null", "object"],
"properties": {
"app_id": {
"type": ["null", "number"]
},
"created_by_user": {
"type": ["null", "object"],
"properties": {
Expand Down
3 changes: 3 additions & 0 deletions tap_pendo/schemas/guides.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
{
"type": ["null", "object"],
"properties": {
"app_id": {
"type": ["null", "number"]
},
"created_by_user": {
"type": ["null", "object"],
"properties": {
Expand Down
3 changes: 3 additions & 0 deletions tap_pendo/schemas/pages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"type": ["null", "object"],
"additional_properties": false,
"properties": {
"app_id": {
"type": ["null", "number"]
},
"created_by_user": {
"type": ["null", "object"],
"properties": {
Expand Down
3 changes: 3 additions & 0 deletions tap_pendo/schemas/poll_events.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"additional_properties": false,
"type": "object",
"properties": {
"app_id": {
"type": ["null", "number"]
},
"account_id": {
"type": ["null", "string"]
},
Expand Down
3 changes: 3 additions & 0 deletions tap_pendo/schemas/track_types.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"type": ["null", "object"],
"additional_properties": false,
"properties": {
"app_id": {
"type": ["null", "number"]
},
"created_by_user": {
"type": ["null", "object"],
"properties": {
Expand Down
12 changes: 6 additions & 6 deletions tap_pendo/streams.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ def get_body(self):
"all-features",
"pipeline": [{
"source": {
"features": None
"features": {"appId": self.config["app_ids"]}
}
}, {
"sort": ["id"]
Expand Down Expand Up @@ -676,7 +676,7 @@ def get_body(self, period, first, end):
"request": {
"pipeline": [{
"source": {
"events": None,
"events": {"appId": self.config["app_ids"]},
"timeSeries": {
"period": period,
"first": first,
Expand Down Expand Up @@ -709,7 +709,7 @@ def get_body(self, period, first):
"request": {
"pipeline": [{
"source": {
"pollEvents": None,
"pollEvents": {"appId": self.config["app_ids"]},
"timeSeries": {
"period": period,
"first": first,
Expand Down Expand Up @@ -824,7 +824,7 @@ def get_body(self):
"name": "all-track-types",
"pipeline": [{
"source": {
"trackTypes": None
"trackTypes": {"appId": self.config["app_ids"]}
}
}, {
"sort": ["id"]
Expand All @@ -849,7 +849,7 @@ def get_body(self):
"all-guides",
"pipeline": [{
"source": {
"guides": None
"guides": {"appId": self.config["app_ids"]}
}
}, {
"sort": ["id"]
Expand All @@ -875,7 +875,7 @@ def get_body(self):
"all-pages",
"pipeline": [{
"source": {
"pages": None
"pages": {"appId": self.config["app_ids"]}
}
}, {
"sort": ["id"]
Expand Down
6 changes: 6 additions & 0 deletions tests/tap_tester/test_all_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ def test_run(self):

# verify all fields for each stream are replicated
self.assertSetEqual(expected_all_keys, actual_all_keys)

# Verify we have more than one app data
# below four streams are independent of the app_id
if stream not in ["accounts", "visitors", "metadata_accounts", "metadata_visitors"]:
records_appid_set = set([message.get('data').get('app_id') for message in messages.get("messages")])
self.assertGreater(len(records_appid_set), 1, msg=f"We have only one app's records for {stream}")
dsprayberry marked this conversation as resolved.
Show resolved Hide resolved



Expand Down
50 changes: 50 additions & 0 deletions tests/unittests/test_app_ids_configurable.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import unittest
from unittest import mock
from argparse import Namespace
from tap_pendo import main


class TestAppIdConfiguration(unittest.TestCase):
@mock.patch("tap_pendo.utils.parse_args", side_effect=lambda required_config_keys: Namespace(config={"start_date": "", "x_pendo_integration_key": "", "period":""}, discover=True))
@mock.patch("tap_pendo.discover_streams", side_effect=lambda config: {})
@mock.patch('tap_pendo.do_discover')
def test_app_ids_not_in_config(self, mocked_do_discover, mocked_discover_stream, mocked_parse_args):
"""
To verify that if app_ids is not in configure file then select all apps and run discover mode.
"""
main()
dbshah1212 marked this conversation as resolved.
Show resolved Hide resolved
config = {'app_ids': 'expandAppIds("*")', 'start_date': '', 'x_pendo_integration_key': '', 'period': ''}
mocked_do_discover.assert_called_with(config)

@mock.patch("tap_pendo.utils.parse_args", side_effect=lambda required_config_keys: Namespace(config={"app_ids": "123, 456","start_date": "", "x_pendo_integration_key": "", "period":""}, discover=True))
@mock.patch("tap_pendo.discover_streams", side_effect=lambda config: {})
@mock.patch('tap_pendo.do_discover')
def test_app_ids_coma_seperated_string_in_config(self, mocked_do_discover, mocked_discover_stream, mocked_parse_args):
"""
To verify that if app_ids is comma seperated string in configure file then get list of those app_ids and run discover mode.
"""
main()
config = {'app_ids': ['123', '456'], 'start_date': '', 'x_pendo_integration_key': '', 'period': ''}
mocked_do_discover.assert_called_with(config)

@mock.patch("tap_pendo.utils.parse_args", side_effect=lambda required_config_keys: Namespace(config={"app_ids": "","start_date": "", "x_pendo_integration_key": "", "period":""}, discover=True))
@mock.patch("tap_pendo.discover_streams", side_effect=lambda config: {})
@mock.patch('tap_pendo.do_discover')
def test_app_ids_empty_in_config(self, mocked_do_discover, mocked_discover_stream, mocked_parse_args):
"""
To verify that if app_ids is blank string or empty string in configure file then select all apps and run discover mode.
"""
main()
config = {'app_ids': 'expandAppIds("*")', 'start_date': '', 'x_pendo_integration_key': '', 'period': ''}
mocked_do_discover.assert_called_with(config)

@mock.patch("tap_pendo.utils.parse_args", side_effect=lambda required_config_keys: Namespace(config={"app_ids": "123, test, test123, 123test, ","start_date": "", "x_pendo_integration_key": "", "period":""}, discover=True))
def test_app_ids_valid_app_ids_with_invalid_app_ids_config(self, mocked_parse_args):
"""
To verify that if app_ids is comma seperated blanks with string in configure file then then raise exception.
"""

with self.assertRaises(Exception) as e:
main()

self.assertEqual(str(e.exception), "Invalid appIDs provided during the configuration:['test', 'test123', '123test', '']", "Not get expected exception")