[Breadth Coverage] Support Databox in Azure CLI cmdlets #1344

jsntcy · 2020-03-03T10:49:08Z

What does this PR do?

Support the key use case for DataBox.
Issue link is [Breadth Coverage] Onboard Databox Commands azure-cli#12222

What's the key use case?
In the case of an offline transfer, the flow for Databox customers is:

Place an order for a device
Track the order (forward and reverse shipment)
A device is shipped to customer
Customer retrieve the credentials to unlock the device
Customer copies the data
Customer ship back the device
Data is transferred to customer storage account

Which commands will be supported?
az databox job create
az databox job update
az databox job show
az databox job cancel
az databox job delete
az databox job list
az databox job list-credentials

Notes
az databox job list-credentials is a special command. Usually only a physical databox device is received, then the credentials for the device can be retrieved by this command. As discussed with service team, I just tested this command manually in service team's test subscription with fixed resource group and databox job and will not include it in automation test.

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

General Guidelines

Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

My extension description/summary conforms to the Extension Summary Guidelines.

azuresdkci · 2020-03-03T10:49:26Z

If this PR is for a new extension or change to an existing extension, use the following to try out the changes in this PR:

docker run -it microsoft/azure-cli:latest
export EXT=<NAME>
pip install --upgrade --target ~/.azure/cliextensions/$EXT "git+https://github.com/jsntcy/azure-cli-extensions.git@bc-databox-final#subdirectory=src/$EXT&egg=$EXT"

yonzhan · 2020-03-03T13:05:07Z

add to S166.

jiasli · 2020-03-05T03:30:10Z

src/databox/azext_databox/_params.py

+             'characters in length and use any alphanumeric and underscore only')
+
+    with self.argument_context('databox job create') as c:
+        c.argument('job_name', job_name_type)


We usually use --name instead of --job-name, --vault-name or --account-name for the object itself. #Resolved

will update it.

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

mmyyrroonn

LGTM in general except for three_id arguments.

mmyyrroonn · 2020-03-05T03:57:46Z

src/databox/azext_databox/_params.py

+        c.argument('location', arg_type=get_location_type(self.cli_ctx), default=None,
+                   validator=get_default_location_from_resource_group)
+        c.argument('tags', tags_type)
+        c.argument('sku_name', arg_type=get_enum_type(['DataBox', 'DataBoxDisk', 'DataBoxHeavy']),


it should be sku #Resolved

will update it

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

mmyyrroonn · 2020-03-05T03:58:28Z

src/databox/azext_databox/_params.py

+        c.argument('postal_code', help='Postal code.')
+        c.argument('company_name', help='Name of the company.')
+        c.extra('storage_account_id', help='The destination storage account resource id.', arg_group='Storage Account')
+        c.extra('staging_storage_account_id',


it should be storage_account_name or storage_account #Resolved

will be updated.

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

mmyyrroonn · 2020-03-05T04:21:11Z

src/databox/azext_databox/_params.py

+        c.argument('country', help='Name of the Country. Ex: US')
+        c.argument('postal_code', help='Postal code.')
+        c.argument('company_name', help='Name of the company.')
+        c.extra('storage_account_id', help='The destination storage account resource id.', arg_group='Storage Account')


same here. Meanwhile, what's the difference between the storage account id and the staging storage account id? #Resolved

will update it

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

mmyyrroonn · 2020-03-05T04:21:47Z

src/databox/azext_databox/_params.py

+        c.extra('staging_storage_account_id',
+                help='The destination storage account id that can be used to copy the vhd for staging.',
+                arg_group='Managed Disk')
+        c.extra('resource_group_id',


what's this used for? #Resolved

Create new resource group if you intend to create managed disks from on-premises VHDs. You can use an existing resource group only if the resource group was created previously when creating a Data Box order for managed disk by Data Box service.

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

mmyyrroonn · 2020-03-05T04:24:55Z

src/databox/azext_databox/custom.py

+                       street_address3=None,
+                       state_or_province=None,
+                       company_name=None):
+    job_resource = get_databox_job(client, resource_group_name, job_name)


it's already a patch method. Why do we need to get it again? #WontFix

It seems the patch method in SDK is not correct. When just pass one property that need to be updated to SDK, it said some other properties cannot be None.

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

mmyyrroonn · 2020-03-05T04:25:25Z

src/databox/azext_databox/_client_factory.py

+    return cf_databox(cli_ctx).jobs
+
+
+def cf_service(cli_ctx, *_):


do we need this client factory? #Resolved

will remove it.

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

mmyyrroonn · 2020-03-05T04:27:45Z

src/databox/azext_databox/tests/latest/test_databox_scenario.py

+                 '--job-name {job_name} '
+                 '--contact-name "Public SDK Test 1" '
+                 '--email-list testing1@microsoft.com ',
+                 checks=[])


can we add some checks here? just like the show command #WontFix

the updated parameters are not returned from command and I checked the update result by the following 'show' command

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

it's quite strange. Is it a long running operation? It should return something. #WontFix

Yes, it returns something, but the response doesn't include updated properties. "Show" command below should cover the update logic.

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

fengzhou-msft · 2020-03-05T05:21:11Z

src/databox/README.rst

+==========================================
+
+This package is for the 'databox' extension.
+i.e. 'az databox'


Please add more details on how to use this extension. You can refer to storage-preview. #Resolved

Thanks, I'll add it.

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

fengzhou-msft · 2020-03-05T05:27:12Z

src/databox/azext_databox/_params.py

+    job_name_type = CLIArgumentType(
+        help='The name of the job resource within the specified resource group. job names must be between 3 and 24 '
+             'characters in length and use any alphanumeric and underscore only')


add options_list=['--name', '-n']? #Resolved

Will update it.

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

fengzhou-msft · 2020-03-09T08:08:38Z

src/databox/azext_databox/_params.py

+        c.argument('contact_name', help='Contact name of the person.')
+        c.argument('phone', help='Phone number of the contact person.')
+        c.argument('mobile', help='Mobile number of the contact person.')
+        c.argument('email_list', help='Space-separated list of Email addresses to be notified about job progress.', nargs='+')
+        c.argument('street_address1', help='Street Address line 1.')
+        c.argument('street_address2', help='Street Address line 2.')
+        c.argument('street_address3', help='Street Address line 3.')
+        c.argument('city', help='Name of the City.')
+        c.argument('state_or_province', help='Name of the State or Province.')
+        c.argument('country', help='Name of the Country. Ex: US')
+        c.argument('postal_code', help='Postal code.')
+        c.argument('company_name', help='Name of the company.')


You can add arg_group for these deilvery information so it's more organized in help message. #Resolved

good suggestions, will update

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

* Support Databox in Azure CLI cmdlets

jsntcy requested review from mmyyrroonn, Juliehzl and yonzhan March 3, 2020 10:49

jsntcy self-assigned this Mar 3, 2020

jsntcy requested review from qianwens, jiasli, zhoxing-ms and haroldrandom March 3, 2020 10:50

yonzhan added this to the S166 milestone Mar 3, 2020

jsntcy changed the title ~~Support Databox in Azure CLI cmdlets~~ [Breadth Coverage]Support Databox in Azure CLI cmdlets Mar 4, 2020

jsntcy changed the title ~~[Breadth Coverage]Support Databox in Azure CLI cmdlets~~ [Breadth Coverage] Support Databox in Azure CLI cmdlets Mar 4, 2020

jsntcy requested review from fengzhou-msft, arrownj and qwordy March 4, 2020 01:52

jsntcy force-pushed the bc-databox-final branch 2 times, most recently from a299798 to 17bd65b Compare March 4, 2020 09:27

jiasli reviewed Mar 5, 2020

View reviewed changes

mmyyrroonn reviewed Mar 5, 2020

View reviewed changes

fengzhou-msft reviewed Mar 5, 2020

View reviewed changes

jsntcy force-pushed the bc-databox-final branch from 7d02f02 to 1d370f1 Compare March 6, 2020 15:40

fengzhou-msft reviewed Mar 9, 2020

View reviewed changes

jsntcy force-pushed the bc-databox-final branch from c0914d1 to 63ab9f4 Compare March 10, 2020 10:52

jsntcy requested review from kalyanaj, panchagnula, temandr, zikalino and zqingqing1 as code owners March 10, 2020 10:52

jsntcy removed request for temandr, kalyanaj, zikalino, panchagnula and zqingqing1 March 10, 2020 10:58

jsntcy force-pushed the bc-databox-final branch from 63ab9f4 to 5b65682 Compare March 10, 2020 13:49

jsntcy added 6 commits April 7, 2020 14:17

Support Databox in Azure CLI cmdlets

55bb2e2

add code owner to avoid linter error

0eff558

add license headers

c4f0a93

resolve review comments

7faa36d

fix CI error: use readme.md instead of readme.rst in setup.py

acf64f3

group arguments

6a1eeba

jsntcy force-pushed the bc-databox-final branch from 5b65682 to 6a1eeba Compare April 7, 2020 06:25

jsntcy added 2 commits April 7, 2020 14:46

add experimental tag

bf59f77

Merge branch 'master' into bc-databox-final

2441a5f

mmyyrroonn approved these changes Apr 7, 2020

View reviewed changes

jsntcy merged commit 5e8de4e into Azure:master Apr 7, 2020

ManuInNZ pushed a commit to ManuInNZ/azure-cli-extensions that referenced this pull request Apr 11, 2020

[Breadth Coverage] Support Databox in Azure CLI cmdlets (Azure#1344)

be2c827

* Support Databox in Azure CLI cmdlets

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Breadth Coverage] Support Databox in Azure CLI cmdlets #1344

[Breadth Coverage] Support Databox in Azure CLI cmdlets #1344

jsntcy commented Mar 3, 2020 •

edited

Loading

azuresdkci commented Mar 3, 2020

yonzhan commented Mar 3, 2020

jiasli Mar 5, 2020 •

edited by jsntcy

Loading

jsntcy Mar 5, 2020

mmyyrroonn left a comment

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

jsntcy Mar 5, 2020

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

jsntcy Mar 6, 2020

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

jsntcy Mar 6, 2020

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

jsntcy Mar 6, 2020 •

edited

Loading

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

jsntcy Mar 5, 2020

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

jsntcy Mar 5, 2020

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

jsntcy Mar 6, 2020

mmyyrroonn Mar 10, 2020 •

edited by jsntcy

Loading

jsntcy Mar 10, 2020

fengzhou-msft Mar 5, 2020 •

edited by jsntcy

Loading

jsntcy Mar 6, 2020

fengzhou-msft Mar 5, 2020 •

edited by jsntcy

Loading

jsntcy Mar 5, 2020

fengzhou-msft Mar 9, 2020 •

edited by jsntcy

Loading

jsntcy Mar 10, 2020

[Breadth Coverage] Support Databox in Azure CLI cmdlets #1344

[Breadth Coverage] Support Databox in Azure CLI cmdlets #1344

Conversation

jsntcy commented Mar 3, 2020 • edited Loading

General Guidelines

azuresdkci commented Mar 3, 2020

yonzhan commented Mar 3, 2020

jiasli Mar 5, 2020 • edited by jsntcy Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmyyrroonn left a comment

Choose a reason for hiding this comment

mmyyrroonn Mar 5, 2020 • edited by jsntcy Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmyyrroonn Mar 5, 2020 • edited by jsntcy Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmyyrroonn Mar 5, 2020 • edited by jsntcy Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmyyrroonn Mar 5, 2020 • edited by jsntcy Loading

Choose a reason for hiding this comment

jsntcy Mar 6, 2020 • edited Loading

Choose a reason for hiding this comment

mmyyrroonn Mar 5, 2020 • edited by jsntcy Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmyyrroonn Mar 5, 2020 • edited by jsntcy Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmyyrroonn Mar 5, 2020 • edited by jsntcy Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmyyrroonn Mar 10, 2020 • edited by jsntcy Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fengzhou-msft Mar 5, 2020 • edited by jsntcy Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fengzhou-msft Mar 5, 2020 • edited by jsntcy Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fengzhou-msft Mar 9, 2020 • edited by jsntcy Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsntcy commented Mar 3, 2020 •

edited

Loading

jiasli Mar 5, 2020 •

edited by jsntcy

Loading

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

jsntcy Mar 6, 2020 •

edited

Loading

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

mmyyrroonn Mar 5, 2020 •

edited by jsntcy

Loading

mmyyrroonn Mar 10, 2020 •

edited by jsntcy

Loading

fengzhou-msft Mar 5, 2020 •

edited by jsntcy

Loading

fengzhou-msft Mar 5, 2020 •

edited by jsntcy

Loading

fengzhou-msft Mar 9, 2020 •

edited by jsntcy

Loading