-
Notifications
You must be signed in to change notification settings - Fork 77
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
fix: remove new exceptions #326
Changes from 2 commits
f97139a
d9f8550
57efb3e
3bfa12f
714dd75
e66b007
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 |
---|---|---|
|
@@ -5,14 +5,14 @@ | |
# This source code is licensed under Apache 2.0 License. | ||
|
||
|
||
import json | ||
import time | ||
|
||
from nebula3.Exception import ( | ||
IOErrorException, | ||
NotValidConnectionException, | ||
ExecutionErrorException, | ||
) | ||
|
||
from nebula3.common.ttypes import ErrorCode | ||
from nebula3.data.ResultSet import ResultSet | ||
from nebula3.gclient.net.AuthResult import AuthResult | ||
from nebula3.logger import logger | ||
|
@@ -25,6 +25,7 @@ def __init__( | |
auth_result: AuthResult, | ||
pool, | ||
retry_connect=True, | ||
retry_execute=False, | ||
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. Let's make the execution error retry opt-in, thus to not surprise anyone. |
||
retry_times=3, | ||
retry_interval_sec=1, | ||
): | ||
|
@@ -35,7 +36,8 @@ def __init__( | |
:param auth_result: The result of the authentication process. | ||
:param pool: The pool object where the session was created. | ||
:param retry_connect: A boolean indicating whether to retry the connection if it fails. | ||
:param retry_times: The number of times to retry the connection. | ||
:param retry_execute: A boolean indicating whether to retry the execution if got execution error(-1005), by default False. | ||
:param retry_times: The number of times to retry the connection/execution. | ||
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. maybe we don't need retry_execute param, 0 retry_times means no retry for execution. and mark retry_times default 0 to keep the same with no-retry mechanism. 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. agree :), the retry times is already a newly introduced and it's feasible to be as flag with 0 value, done |
||
:param retry_interval_sec: The interval between connection retries in seconds. | ||
""" | ||
self._session_id = auth_result.get_session_id() | ||
|
@@ -45,6 +47,7 @@ def __init__( | |
# connection the where the session was created, if session pool was used | ||
self._pool = pool | ||
self._retry_connect = retry_connect | ||
self._retry_execute = retry_execute | ||
self._retry_times = retry_times | ||
self._retry_interval_sec = retry_interval_sec | ||
# the time stamp when the session was added to the idle list of the session pool | ||
|
@@ -57,11 +60,23 @@ def execute_parameter(self, stmt, params): | |
:return: ResultSet | ||
""" | ||
if self._connection is None: | ||
raise RuntimeError('The session has been released') | ||
raise RuntimeError("The session has been released") | ||
try: | ||
start_time = time.time() | ||
resp = self._connection.execute_parameter(self._session_id, stmt, params) | ||
end_time = time.time() | ||
|
||
if self._retry_execute and resp.error_code == ErrorCode.E_EXECUTION_ERROR: | ||
retry_count = 0 | ||
while retry_count < self._retry_times: | ||
time.sleep(self._retry_interval_sec) | ||
resp = self._connection.execute_parameter( | ||
self._session_id, stmt, params | ||
) | ||
if resp.error_code != ErrorCode.E_EXECUTION_ERROR: | ||
break | ||
retry_count += 1 | ||
|
||
return ResultSet( | ||
resp, | ||
all_latency=int((end_time - start_time) * 1000000), | ||
|
@@ -72,7 +87,7 @@ def execute_parameter(self, stmt, params): | |
self._pool.update_servers_status() | ||
if self._retry_connect: | ||
if not self._reconnect(): | ||
logger.warning('Retry connect failed') | ||
logger.warning("Retry connect failed") | ||
raise IOErrorException( | ||
IOErrorException.E_ALL_BROKEN, ie.message | ||
) | ||
|
@@ -86,27 +101,6 @@ def execute_parameter(self, stmt, params): | |
timezone_offset=self._timezone_offset, | ||
) | ||
raise | ||
except ExecutionErrorException as eee: | ||
retry_count = 0 | ||
while retry_count < self._retry_times: | ||
try: | ||
# TODO: add exponential backoff | ||
time.sleep(self._retry_interval_sec) | ||
resp = self._connection.execute_parameter( | ||
self._session_id, stmt, params | ||
) | ||
end_time = time.time() | ||
return ResultSet( | ||
resp, | ||
all_latency=int((end_time - start_time) * 1000000), | ||
timezone_offset=self._timezone_offset, | ||
) | ||
except ExecutionErrorException: | ||
if retry_count >= self._retry_times - 1: | ||
raise eee | ||
else: | ||
retry_count += 1 | ||
continue | ||
except Exception: | ||
raise | ||
|
||
|
@@ -244,18 +238,35 @@ def execute_json_with_parameter(self, stmt, params): | |
:return: JSON string | ||
""" | ||
if self._connection is None: | ||
raise RuntimeError('The session has been released') | ||
raise RuntimeError("The session has been released") | ||
try: | ||
resp_json = self._connection.execute_json_with_parameter( | ||
self._session_id, stmt, params | ||
) | ||
if self._retry_execute: | ||
for retry_count in range(self._retry_times): | ||
if ( | ||
json.loads(resp_json).get("errors", [{}])[0].get("code") | ||
!= ErrorCode.E_EXECUTION_ERROR | ||
): | ||
break | ||
logger.warning( | ||
"Execute failed, retry count:{}/{} in {} seconds".format( | ||
retry_count + 1, self._retry_times, self._retry_interval_sec | ||
) | ||
) | ||
time.sleep(self._retry_interval_sec) | ||
resp_json = self._connection.execute_json_with_parameter( | ||
self._session_id, stmt, params | ||
) | ||
return resp_json | ||
|
||
except IOErrorException as ie: | ||
if ie.type == IOErrorException.E_CONNECT_BROKEN: | ||
self._pool.update_servers_status() | ||
if self._retry_connect: | ||
if not self._reconnect(): | ||
logger.warning('Retry connect failed') | ||
logger.warning("Retry connect failed") | ||
raise IOErrorException( | ||
IOErrorException.E_ALL_BROKEN, ie.message | ||
) | ||
|
@@ -264,22 +275,6 @@ def execute_json_with_parameter(self, stmt, params): | |
) | ||
return resp_json | ||
raise | ||
except ExecutionErrorException as eee: | ||
retry_count = 0 | ||
while retry_count < self._retry_times: | ||
try: | ||
# TODO: add exponential backoff | ||
time.sleep(self._retry_interval_sec) | ||
resp = self._connection.execute_json_with_parameter( | ||
self._session_id, stmt, params | ||
) | ||
return resp | ||
except ExecutionErrorException: | ||
if retry_count >= self._retry_times - 1: | ||
raise eee | ||
else: | ||
retry_count += 1 | ||
continue | ||
except Exception: | ||
raise | ||
|
||
|
@@ -310,7 +305,7 @@ def ping_session(self): | |
return True | ||
else: | ||
logger.error( | ||
'failed to ping the session: error code:{}, error message:{}'.format( | ||
"failed to ping the session: error code:{}, error message:{}".format( | ||
resp.error_code, resp.error_msg | ||
) | ||
) | ||
|
@@ -342,5 +337,5 @@ def _idle_time(self): | |
def _sign_out(self): | ||
"""sign out the session""" | ||
if self._connection is None: | ||
raise RuntimeError('The session has been released') | ||
raise RuntimeError("The session has been released") | ||
self._connection.signout(self._session_id) |
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.
please ignore formatter's change, sorry