Skip to content
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

Added get file range with the prevsharesnapshot parameter #13507

Merged
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from azure.storage.blob import AccessPolicy as BlobAccessPolicy
from azure.storage.blob import DelimitedTextDialect as BlobDelimitedTextDialect
from azure.storage.blob import DelimitedJsonDialect as BlobDelimitedJSON
from azure.storage.blob._generated.models import StorageErrorException
from azure.storage.blob._generated.models import StorageErrorException # pylint: disable=unused-import
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from azure.storage.blob._generated.models import StorageErrorException # pylint: disable=unused-import

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line could be removed, weird that pylint didn't get mad before!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is actually being used here

I'm not sure why it's causing this pylint error so I just ignored it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems on stg74 branch it's not used, while I didn't see pylint error when merging previous prs.
I just resolved conflicts on this pr and removed this line, let's see if CI is happy

from azure.storage.blob._models import ContainerPropertiesPaged
from ._deserialize import return_headers_and_deserialized_path_list
from ._generated.models import Path
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,15 @@
# Licensed under the MIT License. See License.txt in the project root for
# license information.
# --------------------------------------------------------------------------
# pylint: disable=no-self-use
from typing import ( # pylint: disable=unused-import
Tuple, Dict, List,
TYPE_CHECKING
)

from ._models import ShareProperties, DirectoryProperties, FileProperties
from ._shared.response_handlers import deserialize_metadata
from ._generated.models import ShareFileRangeList


def deserialize_share_properties(response, obj, headers):
Expand Down Expand Up @@ -62,3 +68,16 @@ def deserialize_permission_key(response, obj, headers): # pylint: disable=unuse
if response is None or headers is None:
return None
return headers.get('x-ms-file-permission-key', None)


def get_file_ranges_result(ranges):
# type: (ShareFileRangeList) -> Tuple[List[Dict[str, int]], List[Dict[str, int]]]
file_ranges = [] # type: ignore
clear_ranges = [] # type: List
if ranges.ranges:
file_ranges = [
{'start': file_range.start, 'end': file_range.end} for file_range in ranges.ranges] # type: ignore
if ranges.clear_ranges:
clear_ranges = [
{'start': clear_range.start, 'end': clear_range.end} for clear_range in ranges.clear_ranges]
return file_ranges, clear_ranges
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
# Licensed under the MIT License. See License.txt in the project root for
# license information.
# --------------------------------------------------------------------------
# pylint: disable=too-many-lines
# pylint: disable=too-many-lines, too-many-public-methods
import functools
import time
from io import BytesIO
from typing import ( # pylint: disable=unused-import
Optional, Union, IO, List, Dict, Any, Iterable,
Optional, Union, IO, List, Dict, Any, Iterable, Tuple,
TYPE_CHECKING
)

Expand All @@ -33,7 +33,7 @@
from ._parser import _get_file_permission, _datetime_to_str
from ._lease import ShareLeaseClient
from ._serialize import get_source_conditions, get_access_conditions, get_smb_properties, get_api_version
from ._deserialize import deserialize_file_properties, deserialize_file_stream
from ._deserialize import deserialize_file_properties, deserialize_file_stream, get_file_ranges_result
from ._models import HandlesPaged, NTFSAttributes # pylint: disable=unused-import
from ._download import StorageStreamDownloader

Expand Down Expand Up @@ -1093,14 +1093,49 @@ def upload_range_from_url(self, source_url,
except StorageErrorException as error:
process_storage_error(error)

def _get_ranges_options( # type: ignore
self, offset=None, # type: Optional[int]
length=None, # type: Optional[int]
previous_sharesnapshot=None, # type: Optional[Union[str, Dict[str, Any]]]
**kwargs
):
# type: (...) -> Dict[str, Any]
if self.require_encryption or (self.key_encryption_key is not None):
raise ValueError("Unsupported method for encryption.")
access_conditions = get_access_conditions(kwargs.pop('lease', None))

content_range = None
if offset is not None:
if length is not None:
end_range = offset + length - 1 # Reformat to an inclusive range index
content_range = 'bytes={0}-{1}'.format(offset, end_range)
else:
content_range = 'bytes={0}-'.format(offset)
options = {
'sharesnapshot': self.snapshot,
'lease_access_conditions': access_conditions,
'timeout': kwargs.pop('timeout', None),
'range': content_range}
if previous_sharesnapshot:
try:
options['prevsharesnapshot'] = previous_sharesnapshot.snapshot # type: ignore
except AttributeError:
try:
options['prevsharesnapshot'] = previous_sharesnapshot['snapshot'] # type: ignore
except TypeError:
options['prevsharesnapshot'] = previous_sharesnapshot
options.update(kwargs)
return options

@distributed_trace
def get_ranges( # type: ignore
self, offset=None, # type: Optional[int]
length=None, # type: Optional[int]
**kwargs # type: Any
):
# type: (...) -> List[Dict[str, int]]
"""Returns the list of valid ranges of a file.
"""Returns the list of valid page ranges for a file or snapshot
of a file.

:param int offset:
Specifies the start offset of bytes over which to get ranges.
Expand All @@ -1115,31 +1150,62 @@ def get_ranges( # type: ignore
:paramtype lease: ~azure.storage.fileshare.ShareLeaseClient or str
:keyword int timeout:
The timeout parameter is expressed in seconds.
:returns: A list of valid ranges.
:returns:
A list of valid ranges.
:rtype: List[dict[str, int]]
"""
timeout = kwargs.pop('timeout', None)
if self.require_encryption or (self.key_encryption_key is not None):
raise ValueError("Unsupported method for encryption.")
access_conditions = get_access_conditions(kwargs.pop('lease', None))
options = self._get_ranges_options(
offset=offset,
length=length,
**kwargs)
try:
ranges = self._client.file.get_range_list(**options)
except StorageErrorException as error:
process_storage_error(error)
return [{'start': file_range.start, 'end': file_range.end} for file_range in ranges.ranges]

content_range = None
if offset is not None:
if length is not None:
end_range = offset + length - 1 # Reformat to an inclusive range index
content_range = 'bytes={0}-{1}'.format(offset, end_range)
else:
content_range = 'bytes={0}-'.format(offset)
def get_ranges_diff( # type: ignore
self,
previous_sharesnapshot, # type: Union[str, Dict[str, Any]]
offset=None, # type: Optional[int]
length=None, # type: Optional[int]
**kwargs # type: Any
):
# type: (...) -> Tuple[List[Dict[str, int]], List[Dict[str, int]]]
"""Returns the list of valid page ranges for a file or snapshot
of a file.

.. versionadded:: 12.6.0

:param int offset:
Specifies the start offset of bytes over which to get ranges.
:param int length:
Number of bytes to use over which to get ranges.
:param str previous_sharesnapshot:
The snapshot diff parameter that contains an opaque DateTime value that
specifies a previous file snapshot to be compared
against a more recent snapshot or the current file.
:keyword lease:
Required if the file has an active lease. Value can be a ShareLeaseClient object
or the lease ID as a string.
:paramtype lease: ~azure.storage.fileshare.ShareLeaseClient or str
:keyword int timeout:
The timeout parameter is expressed in seconds.
:returns:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return type needs to be changed, it's not a tuple now,

I'm thinking if we want to create a new API to get range diff, sharing the same API with get_ranges could cause confusion I feel.
Since # type: (...) -> Union[List[Dict[str, int]], Tuple[List[Dict[str, int]], List[Dict[str, int]]]] is kinda confusing. Users might not sure if they will get a Tuple or List.
@annatisch @kasobol-msft Do you have any opinion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @xiafu-msft - yes we shouldn't have mixed return types.
So are "cleared ranges" only available when the diff snapshot parameter is provided?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is correct @annatisch

A tuple of two lists of file ranges as dictionaries with 'start' and 'end' keys.
The first element are filled file ranges, the 2nd element is cleared file ranges.
:rtype: tuple(list(dict(str, str), list(dict(str, str))
"""
options = self._get_ranges_options(
offset=offset,
length=length,
previous_sharesnapshot=previous_sharesnapshot,
**kwargs)
try:
ranges = self._client.file.get_range_list(
range=content_range,
sharesnapshot=self.snapshot,
lease_access_conditions=access_conditions,
timeout=timeout,
**kwargs)
ranges = self._client.file.get_range_list(**options)
except StorageErrorException as error:
process_storage_error(error)
return [{'start': b.start, 'end': b.end} for b in ranges]
return get_file_ranges_result(ranges)
tasherif-msft marked this conversation as resolved.
Show resolved Hide resolved

@distributed_trace
def clear_range( # type: ignore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1246,8 +1246,8 @@ async def get_range_list(self, sharesnapshot=None, prevsharesnapshot=None, timeo
~azure.storage.fileshare.models.LeaseAccessConditions
:param callable cls: A custom type or function that will be passed the
direct response
:return: list or the result of cls(response)
:rtype: list[~azure.storage.fileshare.models.Range]
:return: ShareFileRangeList or the result of cls(response)
:rtype: ~azure.storage.fileshare.models.ShareFileRangeList
:raises:
:class:`StorageErrorException<azure.storage.fileshare.models.StorageErrorException>`
"""
Expand Down Expand Up @@ -1296,7 +1296,7 @@ async def get_range_list(self, sharesnapshot=None, prevsharesnapshot=None, timeo
header_dict = {}
deserialized = None
if response.status_code == 200:
deserialized = self._deserialize('[Range]', response)
deserialized = self._deserialize('ShareFileRangeList', response)
header_dict = {
'Last-Modified': self._deserialize('rfc-1123', response.headers.get('Last-Modified')),
'ETag': self._deserialize('str', response.headers.get('ETag')),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,23 @@

try:
from ._models_py3 import AccessPolicy
from ._models_py3 import ClearRange
from ._models_py3 import CopyFileSmbInfo
from ._models_py3 import CorsRule
from ._models_py3 import DirectoryItem
from ._models_py3 import FileHTTPHeaders
from ._models_py3 import FileItem
from ._models_py3 import FileProperty
from ._models_py3 import FileRange
from ._models_py3 import FilesAndDirectoriesListSegment
from ._models_py3 import HandleItem
from ._models_py3 import LeaseAccessConditions
from ._models_py3 import ListFilesAndDirectoriesSegmentResponse
from ._models_py3 import ListHandlesResponse
from ._models_py3 import ListSharesResponse
from ._models_py3 import Metrics
from ._models_py3 import Range
from ._models_py3 import RetentionPolicy
from ._models_py3 import ShareFileRangeList
from ._models_py3 import ShareItem
from ._models_py3 import SharePermission
from ._models_py3 import ShareProperties
Expand All @@ -39,21 +41,23 @@
from ._models_py3 import StorageServiceProperties
except (SyntaxError, ImportError):
from ._models import AccessPolicy
from ._models import ClearRange
from ._models import CopyFileSmbInfo
from ._models import CorsRule
from ._models import DirectoryItem
from ._models import FileHTTPHeaders
from ._models import FileItem
from ._models import FileProperty
from ._models import FileRange
from ._models import FilesAndDirectoriesListSegment
from ._models import HandleItem
from ._models import LeaseAccessConditions
from ._models import ListFilesAndDirectoriesSegmentResponse
from ._models import ListHandlesResponse
from ._models import ListSharesResponse
from ._models import Metrics
from ._models import Range
from ._models import RetentionPolicy
from ._models import ShareFileRangeList
from ._models import ShareItem
from ._models import SharePermission
from ._models import ShareProperties
Expand All @@ -79,21 +83,23 @@

__all__ = [
'AccessPolicy',
'ClearRange',
'CopyFileSmbInfo',
'CorsRule',
'DirectoryItem',
'FileHTTPHeaders',
'FileItem',
'FileProperty',
'FileRange',
'FilesAndDirectoriesListSegment',
'HandleItem',
'LeaseAccessConditions',
'ListFilesAndDirectoriesSegmentResponse',
'ListHandlesResponse',
'ListSharesResponse',
'Metrics',
'Range',
'RetentionPolicy',
'ShareFileRangeList',
'ShareItem',
'SharePermission',
'ShareProperties',
Expand Down