Skip to content

Conversation

@djyou
Copy link

@djyou djyou commented Oct 12, 2016

  1. Removed creating resource group/storage account if user specified ones don't exist.
  2. Updated error handling when deployment fails.
  3. Added registry name/storage account/resource group validation before submitting template.

@djyou
Copy link
Author

djyou commented Oct 12, 2016

/cc @sajayantony @ahmetalpbalkan

Copy link

Choose a reason for hiding this comment

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

Why delete --dosable-admin?

Copy link

Choose a reason for hiding this comment

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

Ah I guess that's the create command sorry.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, admin user is by default disabled, so you can only enable it here.

Copy link

Choose a reason for hiding this comment

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

Doesn't RP already validate this? No need to add a ton of CLI side logic for these things usually. (They go out of sync, hard to rev)

Copy link
Author

Choose a reason for hiding this comment

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

We are not asking the user to provide the resource group for a storage account. We are querying ARM for a storage account and ARM will return None if the storage account doesn't exist. We cannot even make the RP call to storage account client because we don't have resource group information. So we are handling it here. There is no client side logic but a simple ARM call to find out if the storage account exists. Besides, we can have a more friendly error/warning message to point the user to the direction of creating a new storage account.

Copy link

Choose a reason for hiding this comment

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

I wonder what other RPs do for these things. Do they let RP figure it out or check it on client side? What's the guidance here?

1. Removed creating resource group/storage account if user specified ones
don't exist.
2. Updated error handling when deployment fails.
3. Added registry name/storage account/resource group validation before
submitting template.
@djyou djyou merged commit 3043b3d into dev Oct 18, 2016
@djyou djyou deleted the doyou/feedback branch October 18, 2016 23:11
djyou pushed a commit that referenced this pull request Sep 25, 2017
* Initial support for VM and single zone VMSS (#14)

* wire up the initial zone support work

* add output

* add tests

* disable package verifications

* use a private copy of network sdk with zone support (#15)

* Support for zoned public IP. Make global zone_type and zones_type. (#16)

* Support for zoned public IP. Make global zone_type and zones_type.

* Code review feedback.

* apply api version range on vm zone test (#19)

* show zone in the table output (#20)

* install: support to build out a msi installer from local sources (#17)

* doc:add command examples using availability zones (#21)

* Installer: build debian bundle from a local clone (#28)

* skip a few expeneive travis builds

* test: update tests to work with new azure-mgmt-compute with zone support

* network: support zone in network lb create (#37)

* VNet peering examples. (#51)

* Add BrazilUS and Dogfood cloud config files. (#50)

* undo all changes specifically for private repository

* fix help per review feedback

* fix lint error

* use newer nrp sdk version
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.

3 participants