-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[Fileshare] Added support for set share properties including access tier #14355
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
Changes from 10 commits
6c8a0b1
0b0c4a9
49e07f6
dc9ed8f
510464f
8838d40
bb6c1f7
c50f937
5598b19
0ea92cf
de78e82
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 |
|---|---|---|
|
|
@@ -36,7 +36,7 @@ | |
| from ._lease import ShareLeaseClient | ||
|
|
||
| if TYPE_CHECKING: | ||
| from ._models import ShareProperties, AccessPolicy | ||
| from ._models import ShareProperties, AccessPolicy, ShareSetPropertiesOptions | ||
|
|
||
|
|
||
| class ShareClient(StorageAccountHostsMixin): | ||
|
|
@@ -305,6 +305,13 @@ def create_share(self, **kwargs): | |
| Name-value pairs associated with the share as metadata. | ||
| :keyword int quota: | ||
| The quota to be allotted. | ||
| :keyword access_tier: | ||
| Specifies the access tier of the share. | ||
| Possible values: 'TransactionOptimized', 'Hot', 'Cool' | ||
| :paramtype access_tier: str or ~azure.storage.fileshare.models.ShareAccessTier | ||
|
|
||
| .. versionadded:: 12.6.0 | ||
|
|
||
| :keyword int timeout: | ||
| The timeout parameter is expressed in seconds. | ||
| :returns: Share-updated property dict (Etag and last modified). | ||
|
|
@@ -321,6 +328,7 @@ def create_share(self, **kwargs): | |
| """ | ||
| metadata = kwargs.pop('metadata', None) | ||
| quota = kwargs.pop('quota', None) | ||
| access_tier = kwargs.pop('access_tier', None) | ||
| timeout = kwargs.pop('timeout', None) | ||
| headers = kwargs.pop('headers', {}) | ||
| headers.update(add_metadata_headers(metadata)) # type: ignore | ||
|
|
@@ -330,6 +338,7 @@ def create_share(self, **kwargs): | |
| timeout=timeout, | ||
| metadata=metadata, | ||
| quota=quota, | ||
| access_tier=access_tier, | ||
| cls=return_response_headers, | ||
| headers=headers, | ||
| **kwargs) | ||
|
|
@@ -504,6 +513,47 @@ def set_share_quota(self, quota, **kwargs): | |
| return self._client.share.set_properties( # type: ignore | ||
| timeout=timeout, | ||
| quota=quota, | ||
| access_tier=None, | ||
| lease_access_conditions=access_conditions, | ||
| cls=return_response_headers, | ||
| **kwargs) | ||
| except StorageErrorException as error: | ||
| process_storage_error(error) | ||
|
|
||
| @distributed_trace | ||
| def set_share_properties(self, options, **kwargs): | ||
| # type: (ShareSetPropertiesOptions, Any) -> Dict[str, Any] | ||
| """Sets the share properties. | ||
|
|
||
| .. versionadded:: 12.6.0 | ||
|
|
||
| :param options: | ||
|
Contributor
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. We don't need to worry about overload for python, so I think we can list the options as keyword arguments. What do you think @annatisch @kasobol-msft
Contributor
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. IMO sounds good. But I'd defer this to python experts. |
||
| Specifies the properties to set on the share. | ||
| :type options: ~azure.storage.fileshare.models.ShareSetPropertiesOptions | ||
| :keyword int timeout: | ||
| The timeout parameter is expressed in seconds. | ||
| :keyword lease: | ||
| Required if the share has an active lease. Value can be a ShareLeaseClient object | ||
| or the lease ID as a string. | ||
| :returns: Share-updated property dict (Etag and last modified). | ||
| :rtype: dict(str, Any) | ||
|
|
||
| .. admonition:: Example: | ||
|
|
||
| .. literalinclude:: ../samples/file_samples_share.py | ||
| :start-after: [START set_share_properties] | ||
| :end-before: [END set_share_properties] | ||
| :language: python | ||
| :dedent: 12 | ||
| :caption: Sets the share properties. | ||
| """ | ||
| access_conditions = get_access_conditions(kwargs.pop('lease', None)) | ||
| timeout = kwargs.pop('timeout', None) | ||
| try: | ||
| return self._client.share.set_properties( # type: ignore | ||
| timeout=timeout, | ||
| quota=options.quota, | ||
| access_tier=options.access_tier, | ||
| lease_access_conditions=access_conditions, | ||
| cls=return_response_headers, | ||
| **kwargs) | ||
|
|
||
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.
In blob we call it blob_tier, do we want to be consistent and call it's share_tier?
Uh oh!
There was an error while loading. Please reload this page.
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.
I actually think
access_tieris more concise. Since this is now a property of shares we shouldn't have a prefixshare. For example, we call the propertyquotanotshare_quotaoretagnotshare_etag. I believe for future namingaccess_tieris the better naming convention.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.
it's should be blob_access_tier and share_access_tier or access_tier. I know we have convention to prefix things with blob_/share_ somewhere (not sure if this applies here). But we should also make sure that "access_tier" can be easily matched with other SDKs and REST docs.
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.
Looking at https://github.com/Azure/azure-sdk-for-java/pull/15897/files, it appears
access_tierwould be consistent with other SDKsThere 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.
Yeah - I think
access_tieris a good name. No need forshareprefix as it's already onShareProeprties.If we wanted, what we can do in a future Blobs release is to add
access_tierand stop documentingblob_tier- it will always still be there of course so it will never be a breaking change.