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 - 16279 Implement request timeouts #55

Merged
merged 11 commits into from
Jan 5, 2022

Conversation

prijendev
Copy link
Contributor

@prijendev prijendev commented Nov 10, 2021

Description of change

  • Implemented request timeout.
  • Added unittest case for the same.
  • If config parameter does not pass or pass the value "", "0", 0, "0.0" then set timeout to default value 5min
  • Updated the integration test to fix the circleci failure.

Manual QA steps

  • Set small timeout and verify that sync calls are backing off for timeout error.
  • Provide timeout in integer, float, and string format and verify that it's used in the request.

Risks

Rollback steps

  • revert this branch

@prijendev prijendev changed the title Implement request timeouts TDL - 16279 Implement request timeouts Nov 10, 2021
Comment on lines 140 to 141
@patch('requests.Request', side_effect=Timeout)
def test_connection_error_backoff(self, mocked_request, ConnectionError):

Choose a reason for hiding this comment

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

Update above test case as it's for ConnectionError but you mocked the request to raise Timeout. Also, you provided ConnectionError as a mocked argument of time.sleep.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

# stream the bookmark is updated based on the last record. Thus the new state updated based
# on stream.
date_difference = (datetime.datetime.now() - datetime.datetime(2021, 10, 25, 00, 00, 00)).days
timedelta_by_stream = {stream: [(date_difference if stream == 'answers' else 25), 0, 0] # {stream_name: [days, hours, minutes], ...}
for stream in self.expected_streams()}
Copy link
Contributor

Choose a reason for hiding this comment

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

The answers stream sets the bookmark as the date when the sync was run. However, after 25th October 2021, there was no record found for answers stream. Hence, we altered the state to set a bookmark so that it never goes beyond 25th October, however for the forms stream the bookmark is updated based on the last record. Thus the new state is updated based on the stream.

@KrisPersonal KrisPersonal merged commit 80c2c45 into master Jan 5, 2022
@kethan1122 kethan1122 deleted the implement-request-timeouts branch April 28, 2023 09:41
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