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

[Fileshare] Added support for set share properties including access tier #14355

Merged
merged 11 commits into from
Oct 18, 2020

Conversation

tasherif-msft
Copy link
Contributor

No description provided.

@tasherif-msft tasherif-msft added the feature-request This issue requires a new behavior in the product in order be resolved. label Oct 8, 2020
@ghost ghost added the Storage Storage Service (Queues, Blobs, Files) label Oct 8, 2020
@tasherif-msft tasherif-msft changed the title [Fileshare] Added support for set access tier [Fileshare] Added support for set share tier Oct 8, 2020
@scbedd
Copy link
Member

scbedd commented Oct 8, 2020

/azp run python - storage - ci

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@scbedd scbedd closed this Oct 8, 2020
@scbedd scbedd reopened this Oct 8, 2020
@scbedd
Copy link
Member

scbedd commented Oct 8, 2020

/azp run python - storage - ci

@scbedd scbedd closed this Oct 8, 2020
@scbedd scbedd reopened this Oct 8, 2020
@scbedd
Copy link
Member

scbedd commented Oct 8, 2020

/azp where

@azure-pipelines
Copy link

Azure DevOps orgs getting events for this repository:

@scbedd
Copy link
Member

scbedd commented Oct 8, 2020

/azp run python - storage - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tasherif-msft
Copy link
Contributor Author

/azp run python - storage - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tasherif-msft tasherif-msft changed the title [Fileshare] Added support for set share tier [Fileshare] Added support for set share properties Oct 15, 2020
@tasherif-msft tasherif-msft changed the title [Fileshare] Added support for set share properties [Fileshare] Added support for set share properties including access tier Oct 15, 2020

.. versionadded:: 12.6.0

:param options:
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO sounds good. But I'd defer this to python experts.

@@ -350,6 +369,7 @@ def _from_generated(cls, generated):
props.last_modified = generated.properties.last_modified
props.etag = generated.properties.etag
props.quota = generated.properties.quota
props.access_tier = generated.properties.access_tier
Copy link
Contributor

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?

Copy link
Contributor Author

@tasherif-msft tasherif-msft Oct 15, 2020

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 prefix share. For example, we call the property quota not share_quota or etag not share_etag. I believe for future naming access_tier is the better naming convention.

Copy link
Contributor

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.

Copy link
Contributor Author

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 SDKs

Copy link
Member

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 for share prefix as it's already on ShareProeprties.

If we wanted, what we can do in a future Blobs release is to add access_tier and stop documenting blob_tier - it will always still be there of course so it will never be a breaking change.

@tasherif-msft tasherif-msft merged commit 6198e25 into Azure:master Oct 18, 2020
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Oct 20, 2020
…into add_business_multipage_tests

* 'master' of https://github.com/Azure/azure-sdk-for-python: (24 commits)
  samples updates from other branch (Azure#14598)
  [form recognizer] add multipage business card form (Azure#14613)
  Remove extra newline at the end of the file (Azure#14608)
  Update .gitignore (Azure#14609)
  Enable the link check for link verification step. (Azure#14604)
  Switch the content from array to string. (Azure#14576)
  [Fileshare] Added support for set share properties including access tier (Azure#14355)
  [EventHubs & ServiceBus] add python3.9 support (Azure#14301)
  Add parse_key_vault_certificate_id method and tests (Azure#14518)
  Enable Codespaces. (Azure#14564)
  Failed the anchor links with Uppercase. (Azure#14535)
  Sync eng/common directory with azure-sdk-tools for PR 1091 (Azure#14550)
  Only check the touched markdown files in PR for the Verify link step (Azure#14466)
  [formrecognizer] add logic to set page_number on `ContactNames` field (Azure#14552)
  update deps for multiapi (Azure#14534)
  add sample tests for business cards and model compose (Azure#14515)
  Ma accept str for datetime (Azure#14517)
  Fix anchor links so they work when converting to html
  [formrecognizer] initial selection marks (Azure#14024)
  Mypy Compatibilty for EventGrid (Azure#14344)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request This issue requires a new behavior in the product in order be resolved. Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants