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-15863: Implement Request Timeout #173

Merged
merged 19 commits into from
Dec 14, 2021

Conversation

savan-chovatiya
Copy link
Contributor

@savan-chovatiya savan-chovatiya commented Oct 18, 2021

Description of change

TDL-15863: Implement Request Timeout

  • Added timeout in facebook_business initialization.
  • Added backoff for timeout exception for all the functions of sync which makes source call.

Manual QA steps

  • Set small timeout and verified that sync calls are backing off for timeout error.

Risks

Rollback steps

  • revert this branch

@savan-chovatiya savan-chovatiya marked this pull request as draft October 18, 2021 14:21
@savan-chovatiya savan-chovatiya changed the base branch from master to crest-master October 21, 2021 06:33
@savan-chovatiya savan-chovatiya marked this pull request as ready for review November 12, 2021 08:45
@@ -153,6 +157,8 @@ def should_retry_api_error(exception):
return True
elif isinstance(exception, TypeError) and str(exception) == "string indices must be integers":
return True
elif isinstance(exception, Timeout):

Choose a reason for hiding this comment

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

Can we add this in or of one of the above conditions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updates as per suggestion.

hpatel41 and others added 5 commits November 19, 2021 13:24
…r replication key field `date_start` (#172)

* added format as date-time in schema file

* added code coverage

* added check for date format in the bookmark test

* added the check for first sync messages

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>
…streams and TDL-9872: replication keys are not specified as expected in discoverable metadata (#167)

* added valid replication keys in catalog

* modified the code

* TDL-9809: Added replication keys in metadata

* adde code coverage

* Resolved review comments

Co-authored-by: harshpatel4_crest <harsh.patel4@crestdatasys.com>
Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>
#168)

* TDL-7455: Added archived data integration test

* TDL-7455: Updated integration test

* added code coverage

* Resolved review comment

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>
Base automatically changed from crest-master to master November 19, 2021 16:19
@KrisPersonal KrisPersonal merged commit 8286e9b into master Dec 14, 2021
@KrisPersonal KrisPersonal deleted the TDL-15863-implement-request-timeouts branch December 14, 2021 17:25
jesuejunior pushed a commit to sixcodes/tap-facebook that referenced this pull request Mar 17, 2023
* TDL-15863: Added request timeout and unit tests

* TDL-15863: Added unit tests

* TDL-15863: Removed commented code

* TDL-15863: Added request_timeout lookup from config also

* added code coverage

* TDL-15863: Added float type cast for timeout

* Updated comment

* added code change for empty string timeout value from config

* Added empty string handling in request_timeout param

* Resolved review comment

* Added if else for request timeout

* Added backoff for ConnectionError

* TDL-9728: Stream `ads_insights_age_gender` has unexpected datatype for replication key field `date_start` (singer-io#172)

* added format as date-time in schema file

* added code coverage

* added check for date format in the bookmark test

* added the check for first sync messages

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-9809: `forced-replication-method` missing from metadata for some streams and TDL-9872: replication keys are not specified as expected in discoverable metadata	 (singer-io#167)

* added valid replication keys in catalog

* modified the code

* TDL-9809: Added replication keys in metadata

* adde code coverage

* Resolved review comments

Co-authored-by: harshpatel4_crest <harsh.patel4@crestdatasys.com>
Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-7455: Add tap-tester test to verify replication of deleted records	 (singer-io#168)

* TDL-7455: Added archived data integration test

* TDL-7455: Updated integration test

* added code coverage

* Resolved review comment

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>
Co-authored-by: harshpatel4_crest <harsh.patel4@crestdatasys.com>
Co-authored-by: Harsh <80324346+harshpatel4crest@users.noreply.github.com>
Co-authored-by: KrisPersonal <66801357+KrisPersonal@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants