rename to --admin-enabled on update/create for acr operations #1276
Conversation
skkestrel
changed the title
|
@skkestrel #1269 has been merged. |
skkestrel
force-pushed
the
t-kezh/admin-flag
branch
2 times, most recently
from
cd5f062 to
dca0f0c
Compare
skkestrel
changed the title
|
Tests have been updated to match with the changed flag, with a few other description changes to match Swagger. |
djyou
force-pushed
the
t-kezh/admin-flag
branch
from
0624617 to
a6b1f3d
Compare
|
CI known issue, should be fixed #1285. I am restarting it |
| show : Get a container registry. | ||
| update : Update a container registry. | ||
| check-name: Checks whether the container registry name is available for use. | ||
| create : Creates or updates a container registry with the specified parameters. |
There was a problem hiding this comment.
While I recognize the SDK call is "create_or_update", it will confuse users as to why you have a command called create that "creates or updates" and a command called update that just updates--and why they update differently. I would keep the help the same as it was--i.e. that it just creates (and you can then drop the 'with the specified parameters' part because that is implied in all commands).
There was a problem hiding this comment.
I agreed that it might be confusing to the user. This description was meant to match what the actual behavior is. If the user "creates" an existing registry with different properties, it is indeed an update, while in update command, all parameters are optional. VM has similar behaviors in their create command.
There was a problem hiding this comment.
It doesn't update so much as overwrite. Every create in the CLI that is backed by a PUT does the same thing, but we don't advertise that because it will confuse and/or frustrate users. Invariably someone will just use 'create' to update something and then be suprised when some optional parameter was thrown away instead of being left alone.
|
|
||
| Create a container registry | ||
| ------------- | ||
| :: | ||
|
|
||
| Command | ||
| az acr create: Create a container registry. | ||
| az acr create: Creates or updates a container registry with the specified parameters. |
There was a problem hiding this comment.
Same comment here. I think this will confuse and frustrate more than help.
| existing tags. | ||
|
|
||
| --name -n [Required]: The name of the container registry. | ||
| --admin-enabled : The value that indicates whether the admin user is enabled. Allowed |
There was a problem hiding this comment.
You could make this more concise. "Indicates whether the admin user is enabled."
| enable_admin=False): | ||
| '''Create a container registry. | ||
| admin_enabled='false'): | ||
| '''Creates or updates a container registry with the specified parameters. |
There was a problem hiding this comment.
Same here. PUT doesn't really "update" so much as "overwrite existing", which is different from your actual update command.
This PR renames --admin-user-enabled to --admin-enabled on create and update operations.
Usage:
Also removed short flags -a and -s in readme for update operation. Additionally, updates descriptions to match new swagger file.