Skip to content

Commit

Permalink
Merge pull request #463 from zezha-msft/disable-logging-expected-error
Browse files Browse the repository at this point in the history 
Avoid logging expected http errors at ERROR level
  • Loading branch information
@zezha-msft
zezha-msft committed Jun 18, 2018
2 parents e0f8dea + 5c9b2db commit 6a7a252
Show file tree
Hide file tree
Showing 17 changed files with 538 additions and 50 deletions.
27 changes: 21 additions & 6 deletions azure-storage-blob/azure/storage/blob/baseblobservice.py
View file
Expand Up @@ -85,6 +85,10 @@
__version__ as package_version,
)

_CONTAINER_ALREADY_EXISTS_ERROR_CODE = 'ContainerAlreadyExists'
_BLOB_NOT_FOUND_ERROR_CODE = 'BlobNotFound'
_CONTAINER_NOT_FOUND_ERROR_CODE = 'ContainerNotFound'

if sys.version_info >= (3,):
from io import BytesIO
else:
Expand Down Expand Up @@ -622,7 +626,7 @@ def create_container(self, container_name, metadata=None,

if not fail_on_exist:
try:
self._perform_request(request)
self._perform_request(request, expected_errors=[_CONTAINER_ALREADY_EXISTS_ERROR_CODE])
return True
except AzureHttpError as ex:
_dont_fail_on_exist(ex)
Expand Down Expand Up @@ -873,7 +877,7 @@ def delete_container(self, container_name, fail_not_exist=False,

if not fail_not_exist:
try:
self._perform_request(request)
self._perform_request(request, expected_errors=[_CONTAINER_NOT_FOUND_ERROR_CODE])
return True
except AzureHttpError as ex:
_dont_fail_not_exist(ex)
Expand Down Expand Up @@ -1575,10 +1579,21 @@ def exists(self, container_name, blob_name=None, snapshot=None, timeout=None):
'''
_validate_not_none('container_name', container_name)
try:
if blob_name is None:
self.get_container_properties(container_name, timeout=timeout)
else:
self.get_blob_properties(container_name, blob_name, snapshot=snapshot, timeout=timeout)
# make head request to see if container/blob/snapshot exists
request = HTTPRequest()
request.method = 'GET' if blob_name is None else 'HEAD'
request.host_locations = self._get_host_locations(secondary=True)
request.path = _get_path(container_name, blob_name)
request.query = {
'snapshot': _to_str(snapshot),
'timeout': _int_to_str(timeout),
'restype': 'container' if blob_name is None else None,
}

expected_errors = [_CONTAINER_NOT_FOUND_ERROR_CODE] if blob_name is None \
else [_CONTAINER_NOT_FOUND_ERROR_CODE, _BLOB_NOT_FOUND_ERROR_CODE]
self._perform_request(request, expected_errors=expected_errors)

return True
except AzureHttpError as ex:
_dont_fail_not_exist(ex)
Expand Down
12 changes: 10 additions & 2 deletions azure-storage-common/azure/storage/common/_error.py
View file
Expand Up @@ -104,11 +104,19 @@ def _dont_fail_not_exist(error):
def _http_error_handler(http_error):
''' Simple error handler for azure.'''
message = str(http_error)
error_code = None

if 'x-ms-error-code' in http_error.respheader:
message += 'ErrorCode: ' + http_error.respheader['x-ms-error-code']
error_code = http_error.respheader['x-ms-error-code']
message += ' ErrorCode: ' + error_code

if http_error.respbody is not None:
message += '\n' + http_error.respbody.decode('utf-8-sig')
raise AzureHttpError(message, http_error.status)

ex = AzureHttpError(message, http_error.status)
ex.error_code = error_code

raise ex


def _validate_type_bytes(param_name, param):
Expand Down
18 changes: 15 additions & 3 deletions azure-storage-common/azure/storage/common/storageclient.py
View file
Expand Up @@ -14,6 +14,7 @@
import requests
from azure.common import (
AzureException,
AzureHttpError,
)

from ._constants import (
Expand Down Expand Up @@ -209,7 +210,7 @@ def extract_date_and_request_id(retry_context):
else:
return ""

def _perform_request(self, request, parser=None, parser_args=None, operation_context=None):
def _perform_request(self, request, parser=None, parser_args=None, operation_context=None, expected_errors=None):
'''
Sends the request and return response. Catches HTTPError and hands it
to error handler
Expand Down Expand Up @@ -281,7 +282,7 @@ def _perform_request(self, request, parser=None, parser_args=None, operation_con
self.extract_date_and_request_id(retry_context),
response.status,
response.message,
str(request.headers).replace('\n', ''))
str(response.headers).replace('\n', ''))

# Parse and wrap HTTP errors in AzureHttpError which inherits from AzureException
if response.status >= 300:
Expand Down Expand Up @@ -325,6 +326,16 @@ def _perform_request(self, request, parser=None, parser_args=None, operation_con
status_code = retry_context.response.status if retry_context.response is not None else 'Unknown'
timestamp_and_request_id = self.extract_date_and_request_id(retry_context)

# if the http error was expected, we should short-circuit
if isinstance(ex, AzureHttpError) and expected_errors is not None and ex.error_code in expected_errors:
logger.info("%s Received expected http error: "
"%s, HTTP status code=%s, Exception=%s.",
client_request_id_prefix,
timestamp_and_request_id,
status_code,
exception_str_in_one_line)
raise ex

logger.info("%s Operation failed: checking if the operation should be retried. "
"Current retry count=%s, %s, HTTP status code=%s, Exception=%s.",
client_request_id_prefix,
Expand Down Expand Up @@ -376,4 +387,5 @@ def _perform_request(self, request, parser=None, parser_args=None, operation_con
# note: to cover the emulator scenario, the host_location is grabbed
# from request.host_locations(which includes the dev account name)
# instead of request.host(which at this point no longer includes the dev account name)
operation_context.host_location = {retry_context.location_mode: request.host_locations[retry_context.location_mode]}
operation_context.host_location = {
retry_context.location_mode: request.host_locations[retry_context.location_mode]}
35 changes: 28 additions & 7 deletions azure-storage-file/azure/storage/file/fileservice.py
View file
Expand Up @@ -83,6 +83,12 @@
__version__ as package_version,
)

_SHARE_NOT_FOUND_ERROR_CODE = 'ShareNotFound'
_PARENT_NOT_FOUND_ERROR_CODE = 'ParentNotFound'
_RESOURCE_NOT_FOUND_ERROR_CODE = 'ResourceNotFound'
_RESOURCE_ALREADY_EXISTS_ERROR_CODE = 'ResourceAlreadyExists'
_SHARE_ALREADY_EXISTS_ERROR_CODE = 'ShareAlreadyExists'

if sys.version_info >= (3,):
from io import BytesIO
else:
Expand Down Expand Up @@ -648,7 +654,7 @@ def create_share(self, share_name, metadata=None, quota=None,

if not fail_on_exist:
try:
self._perform_request(request)
self._perform_request(request, expected_errors=[_SHARE_ALREADY_EXISTS_ERROR_CODE])
return True
except AzureHttpError as ex:
_dont_fail_on_exist(ex)
Expand Down Expand Up @@ -929,7 +935,7 @@ def delete_share(self, share_name, fail_not_exist=False, timeout=None, snapshot=

if not fail_not_exist:
try:
self._perform_request(request)
self._perform_request(request, expected_errors=[_SHARE_NOT_FOUND_ERROR_CODE])
return True
except AzureHttpError as ex:
_dont_fail_not_exist(ex)
Expand Down Expand Up @@ -977,7 +983,7 @@ def create_directory(self, share_name, directory_name, metadata=None,

if not fail_on_exist:
try:
self._perform_request(request)
self._perform_request(request, expected_errors=_RESOURCE_ALREADY_EXISTS_ERROR_CODE)
return True
except AzureHttpError as ex:
_dont_fail_on_exist(ex)
Expand Down Expand Up @@ -1023,7 +1029,7 @@ def delete_directory(self, share_name, directory_name,

if not fail_not_exist:
try:
self._perform_request(request)
self._perform_request(request, expected_errors=[_RESOURCE_NOT_FOUND_ERROR_CODE])
return True
except AzureHttpError as ex:
_dont_fail_not_exist(ex)
Expand Down Expand Up @@ -1273,12 +1279,27 @@ def exists(self, share_name, directory_name=None, file_name=None, timeout=None,
'''
_validate_not_none('share_name', share_name)
try:
request = HTTPRequest()
request.method = 'HEAD' if file_name is not None else 'GET'
request.host_locations = self._get_host_locations()
request.path = _get_path(share_name, directory_name, file_name)

if file_name is not None:
self.get_file_properties(share_name, directory_name, file_name, timeout=timeout, snapshot=snapshot)
restype = None
expected_errors = [_RESOURCE_NOT_FOUND_ERROR_CODE, _PARENT_NOT_FOUND_ERROR_CODE]
elif directory_name is not None:
self.get_directory_properties(share_name, directory_name, timeout=timeout, snapshot=snapshot)
restype = 'directory'
expected_errors = [_RESOURCE_NOT_FOUND_ERROR_CODE, _SHARE_NOT_FOUND_ERROR_CODE]
else:
self.get_share_properties(share_name, timeout=timeout, snapshot=snapshot)
restype = 'share'
expected_errors = [_SHARE_NOT_FOUND_ERROR_CODE]

request.query = {
'restype': restype,
'timeout': _int_to_str(timeout),
'sharesnapshot': _to_str(snapshot)
}
self._perform_request(request, expected_errors=expected_errors)
return True
except AzureHttpError as ex:
_dont_fail_not_exist(ex)
Expand Down
20 changes: 17 additions & 3 deletions azure-storage-queue/azure/storage/queue/queueservice.py
View file
Expand Up @@ -74,6 +74,8 @@
__version__ as package_version,
)

_QUEUE_ALREADY_EXISTS_ERROR_CODE = 'QueueAlreadyExists'
_QUEUE_NOT_FOUND_ERROR_CODE = 'QueueNotFound'
_HTTP_RESPONSE_NO_CONTENT = 204


Expand Down Expand Up @@ -526,7 +528,8 @@ def _return_request(request):

if not fail_on_exist:
try:
response = self._perform_request(request, parser=_return_request)
response = self._perform_request(request, parser=_return_request,
expected_errors=[_QUEUE_ALREADY_EXISTS_ERROR_CODE])
if response.status == _HTTP_RESPONSE_NO_CONTENT:
return False
return True
Expand Down Expand Up @@ -571,7 +574,7 @@ def delete_queue(self, queue_name, fail_not_exist=False, timeout=None):
request.query = {'timeout': _int_to_str(timeout)}
if not fail_not_exist:
try:
self._perform_request(request)
self._perform_request(request, expected_errors=[_QUEUE_NOT_FOUND_ERROR_CODE])
return True
except AzureHttpError as ex:
_dont_fail_not_exist(ex)
Expand Down Expand Up @@ -644,8 +647,19 @@ def exists(self, queue_name, timeout=None):
:return: A boolean indicating whether the queue exists.
:rtype: bool
'''
_validate_not_none('queue_name', queue_name)

try:
self.get_queue_metadata(queue_name, timeout=timeout)
request = HTTPRequest()
request.method = 'GET'
request.host_locations = self._get_host_locations(secondary=True)
request.path = _get_path(queue_name)
request.query = {
'comp': 'metadata',
'timeout': _int_to_str(timeout),
}

self._perform_request(request, expected_errors=[_QUEUE_NOT_FOUND_ERROR_CODE])
return True
except AzureHttpError as ex:
_dont_fail_not_exist(ex)
Expand Down
32 changes: 28 additions & 4 deletions tests/blob/test_common_blob.py
View file
Expand Up @@ -141,23 +141,47 @@ def test_blob_exists(self):
self.assertTrue(exists)

@record
def test_blob_container_not_exists(self):
def test_blob_not_exists(self):
# Arrange
blob_name = self._get_blob_reference()

# Act
exists = self.bs.exists(self._get_container_reference(), blob_name)
exists = self.bs.exists(self.container_name, blob_name)

# Assert
self.assertFalse(exists)

@record
def test_blob_not_exists(self):
def test_blob_snapshot_exists(self):
# Arrange
blob_name = self._create_block_blob()
snapshot = self.bs.snapshot_blob(self.container_name, blob_name)

# Act
exists = self.bs.exists(self.container_name, blob_name, snapshot.snapshot)

# Assert
self.assertTrue(exists)

@record
def test_blob_snapshot_not_exists(self):
# Arrange
blob_name = self._create_block_blob()

# Act
exists = self.bs.exists(self.container_name, blob_name, "1988-08-18T07:52:31.6690068Z")

# Assert
self.assertFalse(exists)

@record
def test_blob_container_not_exists(self):
# In this case both the blob and container do not exist
# Arrange
blob_name = self._get_blob_reference()

# Act
exists = self.bs.exists(self.container_name, blob_name)
exists = self.bs.exists(self._get_container_reference(), blob_name)

# Assert
self.assertFalse(exists)
Expand Down
22 changes: 17 additions & 5 deletions tests/blob/test_container.py
View file
Expand Up @@ -13,7 +13,7 @@
Include, PublicAccess)
from azure.storage.common import AccessPolicy

from tests.testcase import StorageTestCase, TestMode, record
from tests.testcase import StorageTestCase, TestMode, record, LogCaptured

#------------------------------------------------------------------------------
TEST_CONTAINER_PREFIX = 'container'
Expand Down Expand Up @@ -160,7 +160,11 @@ def test_container_not_exists(self):
container_name = self._get_container_reference()

# Act
exists = self.bs.exists(container_name)
with LogCaptured(self) as log_captured:
exists = self.bs.exists(container_name)

log_as_str = log_captured.getvalue()
self.assertTrue('ERROR' not in log_as_str)

# Assert
self.assertFalse(exists)
Expand Down Expand Up @@ -664,7 +668,11 @@ def test_delete_container_with_non_existing_container(self):
container_name = self._get_container_reference()

# Act
deleted = self.bs.delete_container(container_name)
with LogCaptured(self) as log_captured:
deleted = self.bs.delete_container(container_name)

log_as_str = log_captured.getvalue()
self.assertTrue('ERROR' not in log_as_str)

# Assert
self.assertFalse(deleted)
Expand All @@ -675,8 +683,12 @@ def test_delete_container_with_non_existing_container_fail_not_exist(self):
container_name = self._get_container_reference()

# Act
with self.assertRaises(AzureMissingResourceHttpError):
self.bs.delete_container(container_name, True)
with LogCaptured(self) as log_captured:
with self.assertRaises(AzureMissingResourceHttpError):
self.bs.delete_container(container_name, True)

log_as_str = log_captured.getvalue()
self.assertTrue('ERROR' in log_as_str)

# Assert

Expand Down

0 comments on commit 6a7a252

Please sign in to comment.