From 3f517c96481ae5742f9a5ba115067403588709e8 Mon Sep 17 00:00:00 2001 From: antisch Date: Wed, 16 Oct 2019 10:17:11 -0700 Subject: [PATCH 1/6] Updated close handles --- .../azure/storage/file/_polling.py | 66 --------------- .../azure/storage/file/aio/_polling_async.py | 64 --------------- .../file/aio/directory_client_async.py | 76 +++++++++-------- .../storage/file/aio/file_client_async.py | 69 +++++++++------- .../azure/storage/file/directory_client.py | 81 ++++++++++--------- .../azure/storage/file/file_client.py | 74 +++++++++-------- .../azure-storage-file/tests/test_handle.py | 4 +- 7 files changed, 173 insertions(+), 261 deletions(-) delete mode 100644 sdk/storage/azure-storage-file/azure/storage/file/_polling.py delete mode 100644 sdk/storage/azure-storage-file/azure/storage/file/aio/_polling_async.py diff --git a/sdk/storage/azure-storage-file/azure/storage/file/_polling.py b/sdk/storage/azure-storage-file/azure/storage/file/_polling.py deleted file mode 100644 index d956616c2a71..000000000000 --- a/sdk/storage/azure-storage-file/azure/storage/file/_polling.py +++ /dev/null @@ -1,66 +0,0 @@ -# ------------------------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. See License.txt in the project root for -# license information. -# -------------------------------------------------------------------------- -import logging -import time -from typing import Any, Callable -from azure.core.polling import PollingMethod - -from ._shared.response_handlers import process_storage_error -from ._generated.models import StorageErrorException - - -logger = logging.getLogger(__name__) - - -class CloseHandles(PollingMethod): - - def __init__(self, interval): - self._command = None - self._continuation_token = None - self._exception = None - self.handles_closed = 0 - self.polling_interval = interval - - def _update_status(self): - try: - status = self._command(marker=self._continuation_token) - except StorageErrorException as error: - process_storage_error(error) - self._continuation_token = status.get('marker') - self.handles_closed += status.get('number_of_handles_closed') or 0 - - def initialize(self, command, initial_status, _): # pylint: disable=arguments-differ - # type: (Any, Any, Callable) -> None - self._command = command - self._continuation_token = initial_status['marker'] - self.handles_closed = initial_status['number_of_handles_closed'] - - def run(self): - # type: () -> None - try: - while not self.finished(): - self._update_status() - time.sleep(self.polling_interval) - except Exception as e: - logger.warning(str(e)) - raise - - def status(self): - self._update_status() - return self.handles_closed - - def finished(self): - # type: () -> bool - """Is this polling finished? - :rtype: bool - """ - return self._continuation_token is None - - def resource(self): - # type: () -> Any - if not self.finished: - self._update_status() - return self.handles_closed diff --git a/sdk/storage/azure-storage-file/azure/storage/file/aio/_polling_async.py b/sdk/storage/azure-storage-file/azure/storage/file/aio/_polling_async.py deleted file mode 100644 index faa60276d1af..000000000000 --- a/sdk/storage/azure-storage-file/azure/storage/file/aio/_polling_async.py +++ /dev/null @@ -1,64 +0,0 @@ -# ------------------------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. See License.txt in the project root for -# license information. -# -------------------------------------------------------------------------- - -import asyncio -import logging -from typing import Any, Callable -from azure.core.polling import AsyncPollingMethod - -from .._shared.response_handlers import process_storage_error -from .._generated.models import StorageErrorException - - -logger = logging.getLogger(__name__) - - -class CloseHandlesAsync(AsyncPollingMethod): - - def __init__(self, interval): - self._command = None - self._continuation_token = None - self._exception = None - self.handles_closed = 0 - self.polling_interval = interval - - async def _update_status(self): - try: - status = await self._command(marker=self._continuation_token) - except StorageErrorException as error: - process_storage_error(error) - self._continuation_token = status.get('marker') - self.handles_closed += status.get('number_of_handles_closed') or 0 - - def initialize(self, command, initial_status, _): # pylint: disable=arguments-differ - # type: (Any, Any, Callable) -> None - self._command = command - self._continuation_token = initial_status['marker'] - self.handles_closed = initial_status['number_of_handles_closed'] - - async def run(self): - # type: () -> None - try: - while not self.finished(): - await self._update_status() - await asyncio.sleep(self.polling_interval) - except Exception as e: - logger.warning(str(e)) - raise - - def status(self): - return self.handles_closed - - def finished(self): - # type: () -> bool - """Is this polling finished? - :rtype: bool - """ - return self._continuation_token is None - - def resource(self): - # type: () -> Any - return self.handles_closed diff --git a/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py b/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py index 63021bca89bc..3767d169a376 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py @@ -26,7 +26,6 @@ from .._shared.response_handlers import return_response_headers, process_storage_error from .._deserialize import deserialize_directory_properties from ..directory_client import DirectoryClient as DirectoryClientBase -from ._polling_async import CloseHandlesAsync from .file_client_async import FileClient from .models import DirectoryPropertiesPaged, HandlesPaged @@ -262,50 +261,61 @@ def list_handles(self, recursive=False, **kwargs): page_iterator_class=HandlesPaged) @distributed_trace_async - async def close_handles( - self, handle=None, # type: Union[str, HandleItem] - recursive=False, # type: bool - **kwargs # type: Any - ): - # type: (...) -> int - """Close open file handles. + async def close_handle(self, handle, **kwargs): + # type: (Union[str, HandleItem], Any) -> int + """Close an open file handle. :param handle: - Optionally, a specific handle to close. The default value is '*' - which will attempt to close all open handles. + A specific handle to close. :type handle: str or ~azure.storage.file.Handle - :param bool recursive: - Boolean that specifies if operation should apply to the directory specified by the client, - its files, its subdirectories and their files. Default value is False. :keyword int timeout: The timeout parameter is expressed in seconds. - :returns: The number of file handles that were closed. + :returns: + The total number of handles closed. This could be 0 if the supplied + handle was not found. :rtype: int """ timeout = kwargs.pop('timeout', None) + start_time = time.time() try: handle_id = handle.id # type: ignore except AttributeError: - handle_id = handle or '*' - command = functools.partial( - self._client.directory.force_close_handles, - handle_id, - timeout=timeout, - sharesnapshot=self.snapshot, - recursive=recursive, - cls=return_response_headers, - **kwargs) - try: - start_close = await command() - except StorageErrorException as error: - process_storage_error(error) + handle_id = handle + try_close = True + continuation_token = None + total_handles = 0 + while try_close: + response = await self._client.file.force_close_handles( + handle_id, + timeout=timeout, + marker=continuation_token, + sharesnapshot=self.snapshot, + cls=return_response_headers, + **kwargs + ) + continuation_token = response.get('marker') + try_close = bool(continuation_token) + total_handles += response.get('number_of_handles_closed') + if timeout: + timeout = timeout - (time.time() - start_time) + return total_handles + + @distributed_trace_async + async def close_all_handles(self, recursive=False, **kwargs): + # type: (bool, Any) -> int + """Close any open file handles. + + This operation will block until the service has closed all open handles. - polling_method = CloseHandlesAsync(self._config.copy_polling_interval) - return await async_poller( - command, - start_close, - None, - polling_method) + :param bool recursive: + Boolean that specifies if operation should apply to the directory specified by the client, + its files, its subdirectories and their files. Default value is False. + :keyword int timeout: + The timeout parameter is expressed in seconds. + :returns: The total number of handles closed. + :rtype: int + """ + return await self.close_handle(handle='*', recursive=recursive, **kwargs) @distributed_trace_async async def get_directory_properties(self, **kwargs): diff --git a/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py b/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py index 9c1130ad284a..424a1afbdd29 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py @@ -5,6 +5,7 @@ # -------------------------------------------------------------------------- import functools +import time from io import BytesIO from typing import Optional, Union, IO, List, Dict, Any, Iterable, TYPE_CHECKING # pylint: disable=unused-import @@ -27,7 +28,6 @@ from .._shared.response_handlers import return_response_headers, process_storage_error from .._deserialize import deserialize_file_properties, deserialize_file_stream from ..file_client import FileClient as FileClientBase -from ._polling_async import CloseHandlesAsync from .models import HandlesPaged from .download_async import StorageStreamDownloader @@ -879,40 +879,55 @@ def list_handles(self, **kwargs): page_iterator_class=HandlesPaged) @distributed_trace_async - async def close_handles( - self, - handle=None, # type: Union[str, HandleItem] - **kwargs # type: Any - ): - # type: (...) -> int - """Close open file handles. + async def close_handle(self, handle, **kwargs): + # type: (Union[str, HandleItem], Any) -> int + """Close an open file handle. :param handle: - Optionally, a specific handle to close. The default value is '*' - which will attempt to close all open handles. + A specific handle to close. :type handle: str or ~azure.storage.file.Handle :keyword int timeout: The timeout parameter is expressed in seconds. - :returns: The number of file handles that were closed. + :returns: + The total number of handles closed. This could be 0 if the supplied + handle was not found. :rtype: int """ timeout = kwargs.pop('timeout', None) + start_time = time.time() try: - handle_id = handle.id # type: ignore + handle_id = handle.id # type: ignore except AttributeError: - handle_id = handle or "*" - command = functools.partial( - self._client.file.force_close_handles, - handle_id, - timeout=timeout, - sharesnapshot=self.snapshot, - cls=return_response_headers, - **kwargs - ) - try: - start_close = await command() - except StorageErrorException as error: - process_storage_error(error) + handle_id = handle + try_close = True + continuation_token = None + total_handles = 0 + while try_close: + response = await self._client.file.force_close_handles( + handle_id, + timeout=timeout, + marker=continuation_token, + sharesnapshot=self.snapshot, + cls=return_response_headers, + **kwargs + ) + continuation_token = response.get('marker') + try_close = bool(continuation_token) + total_handles += response.get('number_of_handles_closed') + if timeout: + timeout = timeout - (time.time() - start_time) + return total_handles + + @distributed_trace_async + async def close_all_handles(self, **kwargs): + # type: (Any) -> int + """Close any open file handles. - polling_method = CloseHandlesAsync(self._config.copy_polling_interval) - return await async_poller(command, start_close, None, polling_method) + This operation will block until the service has closed all open handles. + + :keyword int timeout: + The timeout parameter is expressed in seconds. + :returns: The total number of handles closed. + :rtype: int + """ + return await self.close_handle(handle='*', **kwargs) diff --git a/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py b/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py index 8b0e73b3c004..c6ad32c3a463 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py @@ -29,7 +29,6 @@ from ._shared.parser import _str from ._parser import _get_file_permission, _datetime_to_str from ._deserialize import deserialize_directory_properties -from ._polling import CloseHandles from .file_client import FileClient from .models import DirectoryPropertiesPaged, HandlesPaged, NTFSAttributes # pylint: disable=unused-import @@ -367,53 +366,61 @@ def list_handles(self, recursive=False, **kwargs): page_iterator_class=HandlesPaged) @distributed_trace - def close_handles( - self, handle=None, # type: Union[str, HandleItem] - recursive=False, # type: bool - **kwargs # type: Any - ): - # type: (...) -> Any - """Close open file handles. - - This operation may not finish with a single call, so a long-running poller - is returned that can be used to wait until the operation is complete. + def close_handle(self, handle, **kwargs): + # type: (Union[str, HandleItem], Any) -> int + """Close an open file handle. :param handle: - Optionally, a specific handle to close. The default value is '*' - which will attempt to close all open handles. + A specific handle to close. :type handle: str or ~azure.storage.file.Handle - :param bool recursive: - Boolean that specifies if operation should apply to the directory specified by the client, - its files, its subdirectories and their files. Default value is False. :keyword int timeout: The timeout parameter is expressed in seconds. - :returns: A long-running poller to get operation status. - :rtype: ~azure.core.polling.LROPoller + :returns: + The total number of handles closed. This could be 0 if the supplied + handle was not found. + :rtype: int """ timeout = kwargs.pop('timeout', None) + start_time = time.time() try: handle_id = handle.id # type: ignore except AttributeError: - handle_id = handle or '*' - command = functools.partial( - self._client.directory.force_close_handles, - handle_id, - timeout=timeout, - sharesnapshot=self.snapshot, - recursive=recursive, - cls=return_response_headers, - **kwargs) - try: - start_close = command() - except StorageErrorException as error: - process_storage_error(error) + handle_id = handle + try_close = True + continuation_token = None + total_handles = 0 + while try_close: + response = self._client.file.force_close_handles( + handle_id, + timeout=timeout, + marker=continuation_token, + sharesnapshot=self.snapshot, + cls=return_response_headers, + **kwargs + ) + continuation_token = response.get('marker') + try_close = bool(continuation_token) + total_handles += response.get('number_of_handles_closed') + if timeout: + timeout = timeout - (time.time() - start_time) + return total_handles + + @distributed_trace + def close_all_handles(self, recursive=False, **kwargs): + # type: (bool, Any) -> int + """Close any open file handles. + + This operation will block until the service has closed all open handles. - polling_method = CloseHandles(self._config.copy_polling_interval) - return LROPoller( - command, - start_close, - None, - polling_method) + :param bool recursive: + Boolean that specifies if operation should apply to the directory specified by the client, + its files, its subdirectories and their files. Default value is False. + :keyword int timeout: + The timeout parameter is expressed in seconds. + :returns: The total number of handles closed. + :rtype: int + """ + return self.close_handle(handle='*', recursive=recursive, **kwargs) @distributed_trace def get_directory_properties(self, **kwargs): diff --git a/sdk/storage/azure-storage-file/azure/storage/file/file_client.py b/sdk/storage/azure-storage-file/azure/storage/file/file_client.py index 9af4d7a859e5..e975b54c5a37 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/file_client.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/file_client.py @@ -5,6 +5,7 @@ # -------------------------------------------------------------------------- # pylint: disable=too-many-lines import functools +import time from io import BytesIO from typing import ( # pylint: disable=unused-import Optional, Union, IO, List, Dict, Any, Iterable, @@ -32,7 +33,6 @@ from ._shared.parser import _str from ._parser import _get_file_permission, _datetime_to_str from ._deserialize import deserialize_file_properties, deserialize_file_stream -from ._polling import CloseHandles from .models import HandlesPaged, NTFSAttributes # pylint: disable=unused-import from ._shared_access_signature import FileSharedAccessSignature from .download import StorageStreamDownloader @@ -1121,45 +1121,55 @@ def list_handles(self, **kwargs): page_iterator_class=HandlesPaged) @distributed_trace - def close_handles( - self, handle=None, # type: Union[str, HandleItem] - **kwargs # type: Any - ): - # type: (...) -> Any - """Close open file handles. - - This operation may not finish with a single call, so a long-running poller - is returned that can be used to wait until the operation is complete. + def close_handle(self, handle, **kwargs): + # type: (Union[str, HandleItem], Any) -> int + """Close an open file handle. :param handle: - Optionally, a specific handle to close. The default value is '*' - which will attempt to close all open handles. + A specific handle to close. :type handle: str or ~azure.storage.file.Handle :keyword int timeout: The timeout parameter is expressed in seconds. - :returns: A long-running poller to get operation status. - :rtype: ~azure.core.polling.LROPoller + :returns: + The total number of handles closed. This could be 0 if the supplied + handle was not found. + :rtype: int """ timeout = kwargs.pop('timeout', None) + start_time = time.time() try: handle_id = handle.id # type: ignore except AttributeError: - handle_id = handle or '*' - command = functools.partial( - self._client.file.force_close_handles, - handle_id, - timeout=timeout, - sharesnapshot=self.snapshot, - cls=return_response_headers, - **kwargs) - try: - start_close = command() - except StorageErrorException as error: - process_storage_error(error) + handle_id = handle + try_close = True + continuation_token = None + total_handles = 0 + while try_close: + response = self._client.file.force_close_handles( + handle_id, + timeout=timeout, + marker=continuation_token, + sharesnapshot=self.snapshot, + cls=return_response_headers, + **kwargs + ) + continuation_token = response.get('marker') + try_close = bool(continuation_token) + total_handles += response.get('number_of_handles_closed') + if timeout: + timeout = timeout - (time.time() - start_time) + return total_handles + + @distributed_trace + def close_all_handles(self, **kwargs): + # type: (Any) -> int + """Close any open file handles. + + This operation will block until the service has closed all open handles. - polling_method = CloseHandles(self._config.copy_polling_interval) - return LROPoller( - command, - start_close, - None, - polling_method) + :keyword int timeout: + The timeout parameter is expressed in seconds. + :returns: The total number of handles closed. + :rtype: int + """ + return self.close_handle(handle='*', **kwargs) diff --git a/sdk/storage/azure-storage-file/tests/test_handle.py b/sdk/storage/azure-storage-file/tests/test_handle.py index 220e71d679c2..91517bd19635 100644 --- a/sdk/storage/azure-storage-file/tests/test_handle.py +++ b/sdk/storage/azure-storage-file/tests/test_handle.py @@ -186,7 +186,7 @@ def test_close_single_handle(self): self._validate_handles(handles) # Act - num_closed = root.close_handles(handle=handles[0]) + num_closed = root.close_handle(handles[0]) # Assert 1 handle has been closed self.assertEqual(1, num_closed.result()) @@ -205,7 +205,7 @@ def test_close_all_handle(self): self._validate_handles(handles) # Act - num_closed = root.close_handles() + num_closed = root.close_all_handles() total_num_handle_closed = num_closed.result() # Assert at least 1 handle has been closed From 3861ae424ca3bd420224211cbc920f7e7df902d7 Mon Sep 17 00:00:00 2001 From: antisch Date: Thu, 17 Oct 2019 07:53:29 -0700 Subject: [PATCH 2/6] Pylint fixes --- .../azure/storage/file/aio/directory_client_async.py | 2 +- .../azure/storage/file/aio/file_client_async.py | 1 - .../azure-storage-file/azure/storage/file/directory_client.py | 2 +- .../azure-storage-file/azure/storage/file/file_client.py | 1 - 4 files changed, 2 insertions(+), 4 deletions(-) diff --git a/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py b/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py index 3767d169a376..e2598f74c804 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py @@ -5,11 +5,11 @@ # -------------------------------------------------------------------------- import functools +import time from typing import ( # pylint: disable=unused-import Optional, Union, Any, Dict, TYPE_CHECKING ) -from azure.core.polling import async_poller from azure.core.async_paging import AsyncItemPaged from azure.core.tracing.decorator import distributed_trace diff --git a/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py b/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py index 424a1afbdd29..47d3683e47fa 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py @@ -10,7 +10,6 @@ from typing import Optional, Union, IO, List, Dict, Any, Iterable, TYPE_CHECKING # pylint: disable=unused-import import six -from azure.core.polling import async_poller from azure.core.async_paging import AsyncItemPaged from azure.core.tracing.decorator import distributed_trace diff --git a/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py b/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py index c6ad32c3a463..79fbb1bb2fec 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py @@ -5,6 +5,7 @@ # -------------------------------------------------------------------------- import functools +import time from typing import ( # pylint: disable=unused-import Optional, Union, Any, Dict, TYPE_CHECKING ) @@ -16,7 +17,6 @@ from urllib2 import quote, unquote # type: ignore import six -from azure.core.polling import LROPoller from azure.core.paging import ItemPaged from azure.core.tracing.decorator import distributed_trace diff --git a/sdk/storage/azure-storage-file/azure/storage/file/file_client.py b/sdk/storage/azure-storage-file/azure/storage/file/file_client.py index e975b54c5a37..1c38b9efb774 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/file_client.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/file_client.py @@ -19,7 +19,6 @@ from urllib2 import quote, unquote # type: ignore import six -from azure.core.polling import LROPoller from azure.core.paging import ItemPaged from azure.core.tracing.decorator import distributed_trace From 8c12fd14a8d5849620251524362666d1cb89cdf8 Mon Sep 17 00:00:00 2001 From: antisch Date: Thu, 17 Oct 2019 08:35:49 -0700 Subject: [PATCH 3/6] Fix tests --- .../azure/storage/file/aio/directory_client_async.py | 6 +++--- .../azure/storage/file/directory_client.py | 6 +++--- sdk/storage/azure-storage-file/tests/test_handle.py | 5 ++--- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py b/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py index e2598f74c804..8fd4bbbbb892 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py @@ -107,7 +107,7 @@ def get_file_client(self, file_name, **kwargs): :param file_name: The name of the file. :returns: A File Client. - :rtype: ~azure.storage.file.file_client.FileClient + :rtype: ~azure.storage.file.FileClient """ if self.directory_path: file_name = self.directory_path.rstrip('/') + "/" + file_name @@ -125,7 +125,7 @@ def get_subdirectory_client(self, directory_name, **kwargs): :param str directory_name: The name of the subdirectory. :returns: A Directory Client. - :rtype: ~azure.storage.file.aio.directory_client_async.DirectoryClient + :rtype: ~azure.storage.file.aio.DirectoryClient .. admonition:: Example: @@ -285,7 +285,7 @@ async def close_handle(self, handle, **kwargs): continuation_token = None total_handles = 0 while try_close: - response = await self._client.file.force_close_handles( + response = await self._client.directory.force_close_handles( handle_id, timeout=timeout, marker=continuation_token, diff --git a/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py b/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py index 79fbb1bb2fec..4a632d4f9996 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py @@ -230,7 +230,7 @@ def get_subdirectory_client(self, directory_name, **kwargs): :param str directory_name: The name of the subdirectory. :returns: A Directory Client. - :rtype: ~azure.storage.file.directory_client.DirectoryClient + :rtype: ~azure.storage.file.DirectoryClient .. admonition:: Example: @@ -390,7 +390,7 @@ def close_handle(self, handle, **kwargs): continuation_token = None total_handles = 0 while try_close: - response = self._client.file.force_close_handles( + response = self._client.directory.force_close_handles( handle_id, timeout=timeout, marker=continuation_token, @@ -540,7 +540,7 @@ def create_subdirectory( :keyword int timeout: The timeout parameter is expressed in seconds. :returns: DirectoryClient - :rtype: ~azure.storage.file.directory_client.DirectoryClient + :rtype: ~azure.storage.file.DirectoryClient .. admonition:: Example: diff --git a/sdk/storage/azure-storage-file/tests/test_handle.py b/sdk/storage/azure-storage-file/tests/test_handle.py index 91517bd19635..a48450d13b94 100644 --- a/sdk/storage/azure-storage-file/tests/test_handle.py +++ b/sdk/storage/azure-storage-file/tests/test_handle.py @@ -189,7 +189,7 @@ def test_close_single_handle(self): num_closed = root.close_handle(handles[0]) # Assert 1 handle has been closed - self.assertEqual(1, num_closed.result()) + self.assertEqual(1, num_closed) @record def test_close_all_handle(self): @@ -206,10 +206,9 @@ def test_close_all_handle(self): # Act num_closed = root.close_all_handles() - total_num_handle_closed = num_closed.result() # Assert at least 1 handle has been closed - self.assertTrue(total_num_handle_closed > 1) + self.assertTrue(num_closed > 1) # ------------------------------------------------------------------------------ From 27cc403c839f9f780d24dc49e71b31b9dc0e75b7 Mon Sep 17 00:00:00 2001 From: antisch Date: Thu, 17 Oct 2019 15:14:08 -0700 Subject: [PATCH 4/6] Review feedback --- .../file/aio/directory_client_async.py | 46 +++++++++++------ .../storage/file/aio/file_client_async.py | 46 +++++++++++------ .../azure/storage/file/directory_client.py | 46 +++++++++++------ .../azure/storage/file/file_client.py | 49 ++++++++++++------- .../azure-storage-file/tests/test_handle.py | 3 ++ 5 files changed, 128 insertions(+), 62 deletions(-) diff --git a/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py b/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py index 8fd4bbbbb892..b0d74d598f4d 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py @@ -275,30 +275,23 @@ async def close_handle(self, handle, **kwargs): handle was not found. :rtype: int """ - timeout = kwargs.pop('timeout', None) - start_time = time.time() try: handle_id = handle.id # type: ignore except AttributeError: handle_id = handle - try_close = True - continuation_token = None - total_handles = 0 - while try_close: + if handle_id == '*': + raise ValueError("Handle ID '*' is not supported. Use 'close_all_handles' instead.") + try: response = await self._client.directory.force_close_handles( handle_id, - timeout=timeout, - marker=continuation_token, + marker=None, sharesnapshot=self.snapshot, cls=return_response_headers, **kwargs ) - continuation_token = response.get('marker') - try_close = bool(continuation_token) - total_handles += response.get('number_of_handles_closed') - if timeout: - timeout = timeout - (time.time() - start_time) - return total_handles + return response.get('number_of_handles_closed', 0) + except StorageErrorException as error: + process_storage_error(error) @distributed_trace_async async def close_all_handles(self, recursive=False, **kwargs): @@ -315,7 +308,30 @@ async def close_all_handles(self, recursive=False, **kwargs): :returns: The total number of handles closed. :rtype: int """ - return await self.close_handle(handle='*', recursive=recursive, **kwargs) + timeout = kwargs.pop('timeout', None) + start_time = time.time() + + try_close = True + continuation_token = None + total_handles = 0 + while try_close: + try: + response = await self._client.directory.force_close_handles( + handle_id='*', + timeout=timeout, + marker=continuation_token, + sharesnapshot=self.snapshot, + cls=return_response_headers, + **kwargs + ) + except StorageErrorException as error: + process_storage_error(error) + continuation_token = response.get('marker') + try_close = bool(continuation_token) + total_handles += response.get('number_of_handles_closed') + if timeout: + timeout = max(0, timeout - (time.time() - start_time)) + return total_handles @distributed_trace_async async def get_directory_properties(self, **kwargs): diff --git a/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py b/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py index 47d3683e47fa..f6aacc694622 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py @@ -892,30 +892,23 @@ async def close_handle(self, handle, **kwargs): handle was not found. :rtype: int """ - timeout = kwargs.pop('timeout', None) - start_time = time.time() try: handle_id = handle.id # type: ignore except AttributeError: handle_id = handle - try_close = True - continuation_token = None - total_handles = 0 - while try_close: + if handle_id == '*': + raise ValueError("Handle ID '*' is not supported. Use 'close_all_handles' instead.") + try: response = await self._client.file.force_close_handles( handle_id, - timeout=timeout, - marker=continuation_token, + marker=None, sharesnapshot=self.snapshot, cls=return_response_headers, **kwargs ) - continuation_token = response.get('marker') - try_close = bool(continuation_token) - total_handles += response.get('number_of_handles_closed') - if timeout: - timeout = timeout - (time.time() - start_time) - return total_handles + return response.get('number_of_handles_closed', 0) + except StorageErrorException as error: + process_storage_error(error) @distributed_trace_async async def close_all_handles(self, **kwargs): @@ -929,4 +922,27 @@ async def close_all_handles(self, **kwargs): :returns: The total number of handles closed. :rtype: int """ - return await self.close_handle(handle='*', **kwargs) + timeout = kwargs.pop('timeout', None) + start_time = time.time() + + try_close = True + continuation_token = None + total_handles = 0 + while try_close: + try: + response = await self._client.file.force_close_handles( + handle_id='*', + timeout=timeout, + marker=continuation_token, + sharesnapshot=self.snapshot, + cls=return_response_headers, + **kwargs + ) + except StorageErrorException as error: + process_storage_error(error) + continuation_token = response.get('marker') + try_close = bool(continuation_token) + total_handles += response.get('number_of_handles_closed') + if timeout: + timeout = max(0, timeout - (time.time() - start_time)) + return total_handles \ No newline at end of file diff --git a/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py b/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py index 4a632d4f9996..f96b6ecc1317 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py @@ -380,30 +380,23 @@ def close_handle(self, handle, **kwargs): handle was not found. :rtype: int """ - timeout = kwargs.pop('timeout', None) - start_time = time.time() try: handle_id = handle.id # type: ignore except AttributeError: handle_id = handle - try_close = True - continuation_token = None - total_handles = 0 - while try_close: + if handle_id == '*': + raise ValueError("Handle ID '*' is not supported. Use 'close_all_handles' instead.") + try: response = self._client.directory.force_close_handles( handle_id, - timeout=timeout, - marker=continuation_token, + marker=None, sharesnapshot=self.snapshot, cls=return_response_headers, **kwargs ) - continuation_token = response.get('marker') - try_close = bool(continuation_token) - total_handles += response.get('number_of_handles_closed') - if timeout: - timeout = timeout - (time.time() - start_time) - return total_handles + return response.get('number_of_handles_closed', 0) + except StorageErrorException as error: + process_storage_error(error) @distributed_trace def close_all_handles(self, recursive=False, **kwargs): @@ -420,7 +413,30 @@ def close_all_handles(self, recursive=False, **kwargs): :returns: The total number of handles closed. :rtype: int """ - return self.close_handle(handle='*', recursive=recursive, **kwargs) + timeout = kwargs.pop('timeout', None) + start_time = time.time() + + try_close = True + continuation_token = None + total_handles = 0 + while try_close: + try: + response = self._client.directory.force_close_handles( + handle_id='*', + timeout=timeout, + marker=continuation_token, + sharesnapshot=self.snapshot, + cls=return_response_headers, + **kwargs + ) + except StorageErrorException as error: + process_storage_error(error) + continuation_token = response.get('marker') + try_close = bool(continuation_token) + total_handles += response.get('number_of_handles_closed') + if timeout: + timeout = max(0, timeout - (time.time() - start_time)) + return total_handles @distributed_trace def get_directory_properties(self, **kwargs): diff --git a/sdk/storage/azure-storage-file/azure/storage/file/file_client.py b/sdk/storage/azure-storage-file/azure/storage/file/file_client.py index 1c38b9efb774..9112cfd9cac3 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/file_client.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/file_client.py @@ -1130,34 +1130,26 @@ def close_handle(self, handle, **kwargs): :keyword int timeout: The timeout parameter is expressed in seconds. :returns: - The total number of handles closed. This could be 0 if the supplied - handle was not found. + The number of handles closed (this may be 0 if the specified handle was not found). :rtype: int """ - timeout = kwargs.pop('timeout', None) - start_time = time.time() try: handle_id = handle.id # type: ignore except AttributeError: handle_id = handle - try_close = True - continuation_token = None - total_handles = 0 - while try_close: + if handle_id == '*': + raise ValueError("Handle ID '*' is not supported. Use 'close_all_handles' instead.") + try: response = self._client.file.force_close_handles( handle_id, - timeout=timeout, - marker=continuation_token, + marker=None, sharesnapshot=self.snapshot, cls=return_response_headers, **kwargs ) - continuation_token = response.get('marker') - try_close = bool(continuation_token) - total_handles += response.get('number_of_handles_closed') - if timeout: - timeout = timeout - (time.time() - start_time) - return total_handles + return response.get('number_of_handles_closed', 0) + except StorageErrorException as error: + process_storage_error(error) @distributed_trace def close_all_handles(self, **kwargs): @@ -1171,4 +1163,27 @@ def close_all_handles(self, **kwargs): :returns: The total number of handles closed. :rtype: int """ - return self.close_handle(handle='*', **kwargs) + timeout = kwargs.pop('timeout', None) + start_time = time.time() + + try_close = True + continuation_token = None + total_handles = 0 + while try_close: + try: + response = self._client.file.force_close_handles( + handle_id='*', + timeout=timeout, + marker=continuation_token, + sharesnapshot=self.snapshot, + cls=return_response_headers, + **kwargs + ) + except StorageErrorException as error: + process_storage_error(error) + continuation_token = response.get('marker') + try_close = bool(continuation_token) + total_handles += response.get('number_of_handles_closed') + if timeout: + timeout = max(0, timeout - (time.time() - start_time)) + return total_handles diff --git a/sdk/storage/azure-storage-file/tests/test_handle.py b/sdk/storage/azure-storage-file/tests/test_handle.py index a48450d13b94..bbd4c295d633 100644 --- a/sdk/storage/azure-storage-file/tests/test_handle.py +++ b/sdk/storage/azure-storage-file/tests/test_handle.py @@ -186,6 +186,9 @@ def test_close_single_handle(self): self._validate_handles(handles) # Act + with self.assertRaises(ValueError): + root.close_handle('*') + num_closed = root.close_handle(handles[0]) # Assert 1 handle has been closed From 4a94146596f41330b2fe0d24a49ce801bd022ef7 Mon Sep 17 00:00:00 2001 From: antisch Date: Thu, 17 Oct 2019 15:19:21 -0700 Subject: [PATCH 5/6] Missing recursive parameter --- .../azure/storage/file/aio/directory_client_async.py | 2 ++ .../azure-storage-file/azure/storage/file/directory_client.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py b/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py index b0d74d598f4d..c280959c1f95 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py @@ -285,6 +285,7 @@ async def close_handle(self, handle, **kwargs): response = await self._client.directory.force_close_handles( handle_id, marker=None, + recursive=None, sharesnapshot=self.snapshot, cls=return_response_headers, **kwargs @@ -320,6 +321,7 @@ async def close_all_handles(self, recursive=False, **kwargs): handle_id='*', timeout=timeout, marker=continuation_token, + recursive=recursive, sharesnapshot=self.snapshot, cls=return_response_headers, **kwargs diff --git a/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py b/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py index f96b6ecc1317..cc29cddf0581 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py @@ -390,6 +390,7 @@ def close_handle(self, handle, **kwargs): response = self._client.directory.force_close_handles( handle_id, marker=None, + recursive=None, sharesnapshot=self.snapshot, cls=return_response_headers, **kwargs @@ -425,6 +426,7 @@ def close_all_handles(self, recursive=False, **kwargs): handle_id='*', timeout=timeout, marker=continuation_token, + recursive=recursive, sharesnapshot=self.snapshot, cls=return_response_headers, **kwargs From 1a851deb1629be5e6ca9bd997d7e713cfae143e1 Mon Sep 17 00:00:00 2001 From: antisch Date: Thu, 17 Oct 2019 16:02:11 -0700 Subject: [PATCH 6/6] Added release notes --- sdk/storage/azure-storage-file/HISTORY.md | 1 + .../azure/storage/file/aio/directory_client_async.py | 2 +- .../azure/storage/file/aio/file_client_async.py | 4 ++-- .../azure-storage-file/azure/storage/file/directory_client.py | 2 +- .../azure-storage-file/azure/storage/file/file_client.py | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/sdk/storage/azure-storage-file/HISTORY.md b/sdk/storage/azure-storage-file/HISTORY.md index 1a2f24c99adc..fd4e10382d9f 100644 --- a/sdk/storage/azure-storage-file/HISTORY.md +++ b/sdk/storage/azure-storage-file/HISTORY.md @@ -27,6 +27,7 @@ the following APIs: - upload_range_from_url - clear_range - get_ranges +- `FileClient.close_handles` and `DirectoryClient.close_handles` have both been replaced by two functions each; `close_handle(handle)` and `close_all_handles()`. These functions are blocking and return integers (the number of closed handles) rather than polling objects. **New features** diff --git a/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py b/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py index c280959c1f95..f809da28baec 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/aio/directory_client_async.py @@ -330,7 +330,7 @@ async def close_all_handles(self, recursive=False, **kwargs): process_storage_error(error) continuation_token = response.get('marker') try_close = bool(continuation_token) - total_handles += response.get('number_of_handles_closed') + total_handles += response.get('number_of_handles_closed', 0) if timeout: timeout = max(0, timeout - (time.time() - start_time)) return total_handles diff --git a/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py b/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py index 8ff9e1e35c9b..07f09dfc36d6 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/aio/file_client_async.py @@ -927,7 +927,7 @@ async def close_all_handles(self, **kwargs): process_storage_error(error) continuation_token = response.get('marker') try_close = bool(continuation_token) - total_handles += response.get('number_of_handles_closed') + total_handles += response.get('number_of_handles_closed', 0) if timeout: timeout = max(0, timeout - (time.time() - start_time)) - return total_handles \ No newline at end of file + return total_handles diff --git a/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py b/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py index cc29cddf0581..77d8613179f2 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/directory_client.py @@ -435,7 +435,7 @@ def close_all_handles(self, recursive=False, **kwargs): process_storage_error(error) continuation_token = response.get('marker') try_close = bool(continuation_token) - total_handles += response.get('number_of_handles_closed') + total_handles += response.get('number_of_handles_closed', 0) if timeout: timeout = max(0, timeout - (time.time() - start_time)) return total_handles diff --git a/sdk/storage/azure-storage-file/azure/storage/file/file_client.py b/sdk/storage/azure-storage-file/azure/storage/file/file_client.py index 2cc079b22666..8348e8be0f18 100644 --- a/sdk/storage/azure-storage-file/azure/storage/file/file_client.py +++ b/sdk/storage/azure-storage-file/azure/storage/file/file_client.py @@ -1174,7 +1174,7 @@ def close_all_handles(self, **kwargs): process_storage_error(error) continuation_token = response.get('marker') try_close = bool(continuation_token) - total_handles += response.get('number_of_handles_closed') + total_handles += response.get('number_of_handles_closed', 0) if timeout: timeout = max(0, timeout - (time.time() - start_time)) return total_handles