-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[Fileshare] Added support for set share properties including access tier #14355
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
6c8a0b1
added access tiers
tasherif-msft 0b0c4a9
fixed async sample
tasherif-msft 49e07f6
removed comments
tasherif-msft dc9ed8f
added sync test
tasherif-msft 510464f
added all tests
tasherif-msft 8838d40
docstrings
tasherif-msft bb6c1f7
reset checks
tasherif-msft c50f937
pylint
tasherif-msft 5598b19
changed method to set share properties
tasherif-msft 0ea92cf
forgot to comment out
tasherif-msft de78e82
removed ShareSetPropertiesOptions
tasherif-msft File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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?
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_tier
is more concise. Since this is now a property of shares we shouldn't have a prefixshare
. For example, we call the propertyquota
notshare_quota
oretag
notshare_etag
. I believe for future namingaccess_tier
is 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_tier
would 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_tier
is a good name. No need forshare
prefix as it's already onShareProeprties
.If we wanted, what we can do in a future Blobs release is to add
access_tier
and stop documentingblob_tier
- it will always still be there of course so it will never be a breaking change.