Skip to content

Conversation

@jsntcy
Copy link
Member

@jsntcy jsntcy commented Dec 6, 2019

This PR is to support #11423 which added the following two commands:

  • az storage account blob-service-properties show: Gets the properties of a storage account’s Blob service.
  • az storage account blob-service-properties update --enable-change-feed: Enable/disable the change feed of a storage account’s Blob service.

TODO: support updating other properties besides 'enable-change-feed'


This checklist is used to make sure that common guidelines for a pull request are followed.

  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

  • I adhere to the Command Guidelines.

@yonzhan yonzhan added this to the S162 milestone Dec 6, 2019
@yonzhan
Copy link
Collaborator

yonzhan commented Dec 6, 2019

@Juliehzl please help review the PR.


with self.argument_context('storage account blob-service-properties update') as c:
c.argument('account_name', acct_name_type, id_part=None)
c.argument('enable_change_feed', arg_type=get_three_state_flag(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Please give more detailed help message for ChangeFeed feature and use active voice.

Copy link
Member Author

Choose a reason for hiding this comment

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

Add help in _help.py


In reply to: 354645627 [](ancestors = 354645627)


with self.command_group('storage account blob-service-properties', blob_service_mgmt_sdk,
custom_command_type=storage_account_custom_type,
resource_type=ResourceType.MGMT_STORAGE, min_api='2019-04-01') as g:
Copy link
Contributor

Choose a reason for hiding this comment

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

Blob Service Properties are supported for a long time, and the correct min api version constraint for blob service properties should be 2018-07-01 I think.
But for changefeed feature it should be 2019-04-01.

Copy link
Member Author

@jsntcy jsntcy Dec 6, 2019

Choose a reason for hiding this comment

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

updated. @[email protected] Please help review again.


In reply to: 354647458 [](ancestors = 354647458)

@jiasli
Copy link
Member

jiasli commented Dec 6, 2019

Help message for update is missing.

>az storage account blob-service-properties -h

Group
    az storage account blob-service-properties

Commands:
    show   : Gets the properties of a storage account’s Blob service, including properties for
             Storage Analytics and CORS (Cross-Origin Resource Sharing) rules.
    update

@jiasli
Copy link
Member

jiasli commented Dec 6, 2019

For az storage account blob-service-properties show -n {}, --group should be optional, like how az storage account show does.

@jiasli
Copy link
Member

jiasli commented Dec 6, 2019

Please run azdev style storage to make sure Linter and Style pass.

@jsntcy
Copy link
Member Author

jsntcy commented Dec 6, 2019

Updated.


In reply to: 562439357 [](ancestors = 562439357)

@jsntcy
Copy link
Member Author

jsntcy commented Dec 6, 2019

Updated. @[email protected] , please help review again.


In reply to: 562439630 [](ancestors = 562439630)

@jsntcy jsntcy requested a review from haroldrandom December 6, 2019 08:10

* GA Release Large File Shares property for storage account create and update command
* GA Release User Delegation SAS token Support
* Add new commands to manage blob service properties for storage account.
Copy link
Member

Choose a reason for hiding this comment

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

How about we put the command group here: az storage account blob-service-properties

Copy link
Member Author

@jsntcy jsntcy Dec 6, 2019

Choose a reason for hiding this comment

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

Do we really need put so specific information here? Do we have a guideline on how to write history?

Copy link
Member

Choose a reason for hiding this comment

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

Not really. We usually refer to the previous history entries.

@haroldrandom
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jsntcy
Copy link
Member Author

jsntcy commented Dec 10, 2019

@[email protected] , what's '/azp run' meaning?


In reply to: 563201902 [](ancestors = 563201902)

@haroldrandom
Copy link
Contributor

@[email protected] , what's '/azp run' meaning?

In reply to: 563201902 [](ancestors = 563201902)

Manually rerun Azure DevOps from here.

@jsntcy jsntcy merged commit 495a6f6 into Azure:dev Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants