Skip to content

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.

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