-
Notifications
You must be signed in to change notification settings - Fork 30
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-20356 update function based to class based #51
base: TDL-20357-add-missing-tap-tester-tests
Are you sure you want to change the base?
TDL-20356 update function based to class based #51
Conversation
…ub.com/singer-io/tap-freshdesk into TDL-20356-update-function-based-to-class-based
…ub.com/singer-io/tap-freshdesk into TDL-20356-update-function-based-to-class-based
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add tap-tester to check independent behaviour of parent-child streams and currently_sycning behaviour of streams (Reference) .
…s://github.com/singer-io/tap-freshdesk into TDL-20356-update-function-based-to-class-based
tap_freshdesk/client.py
Outdated
BASE_URL = "https://{}.freshdesk.com" | ||
|
||
|
||
def ratelimit(limit, every): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is already present under singer-python, so please import it and use it directly.
Reference: https://github.com/singer-io/singer-python/blob/master/singer/utils.py#L81-L99
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the defined function from the tap and used the pre-defined singer functions for rate limit
…-tests' into TDL-20356-update-function-based-to-class-based
…update-function-based-to-class-based
This function will get page size from config, | ||
and will return the default value if an invalid page size is given. | ||
""" | ||
page_size = self.config.get('page_size') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
page_size = self.config.get('page_size') | |
page_size = self.config.get('page_size', DEFAULT_PAGE_SIZE) |
We can remove the below condition -
if page_size is None: return DEFAULT_PAGE_SIZE
expected_list_3 = [{"name": "Agency", "value": "justice league"}, | ||
{"name": "Department", "value": "superhero"}] | ||
|
||
@ parameterized.expand([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ parameterized.expand([ | |
@parameterized.expand([ |
Typo
Description of change
Manual QA steps
Risks
Rollback steps