GL-Work #21
GL-Work #21
Conversation
There was a problem hiding this comment.
On my local testing I observed couple of issue,
[Logon] [InvalidLogonAttempt] Invalid Credentials Provided.is getting retried 5 times, shouldn't be retried.- sync implementation needs to be revisited.
| "zeep", | ||
| 'backoff==1.8.0', | ||
| 'pendulum==1.2.0', | ||
| "singer-python==6.1.1", |
There was a problem hiding this comment.
Please make below changes,
packages=find_packages(),
package_data = {
"tap_listrak/schemas": ["*.json"]
},
There was a problem hiding this comment.
Couple observations needs the attenssion,
- IMO, InvalidLogonAttempt shouldn't be retried.
ERROR Giving up request(...) after 5 tries (zeep.exceptions.Fault: GetContactListCollection(): [Logon] [InvalidLogonAttempt] Invalid Credentials Provided)
- sync implementation needs to be revisited.
…valid credentials, refined sync logic
|
|
This is done. in init.py: Refactored sync() function to dynamically call the sync function for each selected stream insted of hardcoding sync_lists(), |
|
Why did we delete circle ci config? |
tap_listrak/http.py
Outdated
| max_tries=5, | ||
| jitter=None, | ||
| on_backoff=log_retry_attempt, | ||
| giveup=is_non_retriable_exception # This will prevent retry on bad login |
There was a problem hiding this comment.
Since the giveup handler is named is_non_retriable_exception(), the comment doesn't add any value and seems redundant. IMO, no comment is needed here.
There was a problem hiding this comment.
Thank you for your feedback
Removed the comment, and also avoided force push.
Also added the Circle CI config, also added unit tests for the same.
Description of change
This is Combined PR of:
Manual QA steps
Risks
#Rollback steps