-
Notifications
You must be signed in to change notification settings - Fork 53
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-16984: Implement request timeouts #150
base: master
Are you sure you want to change the base?
Changes from 7 commits
4656646
fda173d
1dd1f66
cca2328
02ed1b7
727729a
a3a63b7
028ac34
480f6b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,27 @@ | |
# We need to hold onto this for self-signed SSL | ||
match_hostname = ssl.match_hostname | ||
|
||
def get_request_timeout(): | ||
args = singer.utils.parse_args([]) | ||
# get the value of request timeout from config | ||
config_request_timeout = args.config.get("request_timeout") | ||
|
||
# return default value if timeout from config is none or empty | ||
if not config_request_timeout: | ||
return READ_TIMEOUT_SECONDS | ||
|
||
if isinstance(config_request_timeout, int): # pylint: disable=no-else-return | ||
# return value from config | ||
return config_request_timeout | ||
elif isinstance(config_request_timeout, str) and config_request_timeout.isdigit(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @harshpatel4crest Why is this check not similar to other taps where we check that, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MySQL does not support float values for |
||
# return default value if timeout from config is "0" and integer casted value of valid value | ||
return int(config_request_timeout) if int(config_request_timeout) else READ_TIMEOUT_SECONDS | ||
|
||
# raise Exception as MySql dose not support float values | ||
# Document: https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_net_read_timeout | ||
raise Exception("Unsupported value of timeout, please use string or integer type values.") | ||
|
||
|
||
@backoff.on_exception(backoff.expo, | ||
(pymysql.err.OperationalError), | ||
max_tries=5, | ||
|
@@ -36,7 +57,7 @@ def connect_with_backoff(connection): | |
warnings.append('Could not set session.wait_timeout. Error: ({}) {}'.format(*e.args)) | ||
|
||
try: | ||
cur.execute("SET @@session.net_read_timeout={}".format(READ_TIMEOUT_SECONDS)) | ||
cur.execute("SET @@session.net_read_timeout={}".format(get_request_timeout())) | ||
except pymysql.err.InternalError as e: | ||
warnings.append('Could not set session.net_read_timeout. Error: ({}) {}'.format(*e.args)) | ||
|
||
|
@@ -91,7 +112,7 @@ def __init__(self, config): | |
"port": int(config["port"]), | ||
"cursorclass": config.get("cursorclass") or pymysql.cursors.SSCursor, | ||
"connect_timeout": CONNECT_TIMEOUT_SECONDS, | ||
"read_timeout": READ_TIMEOUT_SECONDS, | ||
"read_timeout": get_request_timeout(), | ||
"charset": "utf8", | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,8 @@ | |
|
||
import copy | ||
import datetime | ||
import functools | ||
import backoff | ||
import singer | ||
import time | ||
import tzlocal | ||
|
@@ -47,6 +49,37 @@ def monkey_patch_date(date_str): | |
#-------------------------------------------------------------------------------------------- | ||
#-------------------------------------------------------------------------------------------- | ||
|
||
# boolean function to check if the error is 'timeout' error or not | ||
def is_timeout_error(): | ||
""" | ||
This function checks whether the URLError contains 'timed out' substring and return boolean | ||
values accordingly, to decide whether to backoff or not. | ||
""" | ||
def gen_fn(exc): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need a nested function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Used 1 function for giveup condition. |
||
if str(exc).__contains__('timed out'): | ||
# retry if the error string contains 'timed out' | ||
return False | ||
return True | ||
|
||
return gen_fn | ||
|
||
def reconnect(details): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why we need this? please add code comment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added comment in the code. |
||
# get connection as 1st param will be 'self' and reconnect | ||
connection = details.get("args")[0].connection | ||
connection.ping(reconnect=True) | ||
|
||
def backoff_timeout_error(fnc): | ||
@backoff.on_exception(backoff.expo, | ||
(pymysql.err.OperationalError), | ||
giveup=is_timeout_error(), | ||
on_backoff=reconnect, | ||
max_tries=5, | ||
factor=2) | ||
@functools.wraps(fnc) | ||
def wrapper(*args, **kwargs): | ||
return fnc(*args, **kwargs) | ||
return wrapper | ||
|
||
def escape(string): | ||
if '`' in string: | ||
raise Exception("Can't escape identifier {} because it contains a backtick" | ||
|
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.
Why we are considering int instead of float?
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.
MySql does not support float values. Document: https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_net_read_timeout.
Added the same comment at line no. 35