Skip to content

Commit

Permalink
Fix so that each request gets a new request id
Browse files Browse the repository at this point in the history
Fixed issue so that a new request id is created for each request
Removed non-deterministic test assertions
  • Loading branch information
gmfeinberg authored Aug 27, 2021
1 parent c54de04 commit 9d29af4
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 19 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ _______

* Cloud only: updated OCI regions

Fixed
_____

* Fixed handling of request id so that each request now gets a new id

====================
5.2.4 - 2021-05-19
====================
Expand Down
16 changes: 15 additions & 1 deletion src/borneo/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def __init__(self, config, logger):
self._max_content_length = (
Client.DEFAULT_MAX_CONTENT_LENGTH if max_content_length == 0
else max_content_length)
self._max_request_id = 1
self._request_id = 1
self._proxy_host = config.get_proxy_host()
self._proxy_port = config.get_proxy_port()
self._proxy_username = config.get_proxy_username()
Expand Down Expand Up @@ -215,12 +215,26 @@ def execute(self, request):
self._config.get_default_compartment())
if self._logutils.is_enabled_for(DEBUG):
self._logutils.log_debug('Request: ' + request.__class__.__name__)
request_id = self._next_request_id()
headers[HttpConstants.REQUEST_ID_HEADER] = request_id
# TODO: look at avoiding creating this object on each request
request_utils = RequestUtils(
self._sess, self._logutils, request, self._retry_handler, self,
self._rate_limiter_map)
return request_utils.do_post_request(
self._request_uri, headers, content, timeout_ms)


@synchronized
def _next_request_id(self):
"""
Get the next client-scoped request id. It really needs to be combined
with a client id to obtain a globally unique scope but is sufficient
for most purposes
"""
self._request_id += 1
return str(self._request_id)

def get_auth_provider(self):
return self._auth_provider

Expand Down
25 changes: 10 additions & 15 deletions src/borneo/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ class RequestUtils(object):
def __init__(self, sess, logutils, request=None, retry_handler=None,
client=None, rate_limiter_map=None):
"""
Init the RequestUtils.
Init the RequestUtils. There are 2 users of this class:
1. Normal requests to the proxy. In this case a new instance of
this class is created for every request
2. KV-specific HTTP requests for login/logout/etc when using
a secure store. In this case there is no request and the same
RequestUtils instance is reused for all requests
:param sess: the session.
:type sess: Session
Expand All @@ -71,7 +76,6 @@ def __init__(self, sess, logutils, request=None, retry_handler=None,
self._retry_handler = retry_handler
self._client = client
self._rate_limiter_map = rate_limiter_map
self._max_request_id = 1
self._auth_provider = (
None if client is None else client.get_auth_provider())
self.lock = Lock()
Expand Down Expand Up @@ -250,10 +254,10 @@ def _do_request(self, method, uri, headers, payload, timeout_ms):
self._log_retried(num_retried, exception)
response = None
try:
if self._request is not None:
request_id = str(self._next_request_id())
headers[HttpConstants.REQUEST_ID_HEADER] = request_id
elif payload is not None:
# this logic is accounting for the fact that there may
# be kv requests that do not have a request instance, and
# only contain a payload
if self._request is None and payload is not None:
payload = payload.encode()
if payload is None:
response = self._sess.request(
Expand Down Expand Up @@ -462,15 +466,6 @@ def _log_retried(self, num_retried, exception):
('' if exception is None else ', exception: ' + str(exception)))
self._logutils.log_debug(msg)

@synchronized
def _next_request_id(self):
"""
Get the next client-scoped request id. It needs to be combined with the
client id to obtain a globally unique scope.
"""
self._max_request_id += 1
return self._max_request_id

def _process_response(self, request, content, status):
"""
Convert the url_response into a suitable return value.
Expand Down
3 changes: 0 additions & 3 deletions test/system_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ def testSystemStatusRequestNormal(self):
# show the status of the create namespace system request.
self.sys_status.set_operation_id(result.get_operation_id())
result = self.handle.system_status(self.sys_status)
self.check_system_result(result, SystemState.WORKING, True)
result.wait_for_completion(self.handle, wait_timeout, 1000)
self.check_system_result(result, SystemState.COMPLETE, True)
# execute drop namespace system request.
Expand All @@ -96,8 +95,6 @@ def testSystemStatusRequestNormal(self):
self.sys_status.set_operation_id(
result.get_operation_id()).set_statement(self.create)
result = self.handle.system_status(self.sys_status)
self.check_system_result(result, SystemState.WORKING, True,
statement=self.create)
result.wait_for_completion(self.handle, wait_timeout, 1000)
self.check_system_result(result, SystemState.COMPLETE, True,
statement=self.create)
Expand Down

0 comments on commit 9d29af4

Please sign in to comment.