pscloud: Release stable version 1.0.0 with CLI improvements #9488
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,10 @@ | |
| Release History | ||
| =============== | ||
|
|
||
| 1.0.0 | ||
| +++++ | ||
| * Stable release. | ||
|
|
||
| 1.0.0b1 | ||
| ++++++ | ||
| * Initial release. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,6 @@ | |
|
|
||
| @register_command( | ||
| "pscloud show", | ||
| ) | ||
| class Show(AAZCommand): | ||
| """Get a reservation | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,4 +16,3 @@ | |
| from ._list import * | ||
| from ._show import * | ||
| from ._update import * | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -13,13 +13,12 @@ | |||||
|
|
||||||
| @register_command( | ||||||
| "pscloud pool create", | ||||||
| ) | ||||||
| class Create(AAZCommand): | ||||||
| """Create a storage pool | ||||||
|
|
||||||
| :example: StoragePools_Create | ||||||
| az pscloud pool create --resource-group rgpurestorage --storage-pool-name storagePoolname --zone 1 --subnet-name /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/virtualNetworks/{vnetName}/subnets/{subnetName} --vnet-name /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/virtualNetworks/{vnetName} --provisioned-bandwidth 100 --reservation-id /subscriptions/{subscriptionId}/providers/PureStorage.Block/reservations/{reservationName} --location eastus | ||||||
|
There was a problem hiding this comment. The example command in the code uses
Suggested change
Contributor
Author
There was a problem hiding this comment. No changes made in create, changes regarding storage pool name have only been made in get-avs-status and get-health status. |
||||||
| """ | ||||||
|
|
||||||
| _aaz_info = { | ||||||
|
|
@@ -60,58 +59,25 @@ def _build_arguments_schema(cls, *args, **kwargs): | |||||
| ), | ||||||
| ) | ||||||
|
|
||||||
| # define Arg Group "Properties" | ||||||
|
|
||||||
| _args_schema = cls._args_schema | ||||||
| _args_schema.zone = AAZStrArg( | ||||||
| options=["-z", "--zone"], | ||||||
| arg_group="Properties", | ||||||
| help="Azure Availability Zone the Pool is located in", | ||||||
| required=True, | ||||||
| ) | ||||||
| _args_schema.provisioned_bandwidth = AAZIntArg( | ||||||
| options=["--provisioned-bandwidth"], | ||||||
| arg_group="Properties", | ||||||
| help="Total bandwidth provisioned for the pool, in MB/s", | ||||||
| required=True, | ||||||
| ) | ||||||
| _args_schema.reservation_id = AAZStrArg( | ||||||
| options=["--reservation-id"], | ||||||
| arg_group="Properties", | ||||||
| help="Azure resource ID of the Pure Storage Cloud service (reservation resource) this storage pool belongs to", | ||||||
| required=True, | ||||||
| ) | ||||||
|
|
||||||
|
|
@@ -134,6 +100,22 @@ def _build_arguments_schema(cls, *args, **kwargs): | |||||
|
|
||||||
| tags = cls._args_schema.tags | ||||||
| tags.Element = AAZStrArg() | ||||||
|
|
||||||
| # define Arg Group "VnetInjection" | ||||||
|
|
||||||
| _args_schema = cls._args_schema | ||||||
| _args_schema.subnet_id = AAZStrArg( | ||||||
| options=["--subnet-name"], | ||||||
| arg_group="VnetInjection", | ||||||
| help="Azure resource ID of the Virtual Network subnet where the storage pool will be connected", | ||||||
| required=True, | ||||||
| ) | ||||||
| _args_schema.vnet_id = AAZStrArg( | ||||||
| options=["--vnet-name"], | ||||||
| arg_group="VnetInjection", | ||||||
| help="Azure resource ID of the Virtual Network in which the subnet is located", | ||||||
| required=True, | ||||||
| ) | ||||||
|
Comment on lines
+107
to
+118
There was a problem hiding this comment. The parameter option names are misleading and inconsistent with the help text and actual values expected. The parameter options are named The documentation in lines 90-91 and 102-103 of README.md also shows conflicting information - they suggest these should be simple names like "mySubnet" and "myVnet", but line 11 and 19 mention Either:
Based on the PR description mentioning "Network parameter simplification: Use --subnet-name and --vnet-name instead of complex JSON objects", it appears the intention is to use names, but the help text needs to be corrected. |
||||||
| return cls._args_schema | ||||||
|
|
||||||
| def _execute_operations(self): | ||||||
|
|
@@ -242,26 +224,16 @@ def content(self): | |||||
| typ=AAZObjectType, | ||||||
| typ_kwargs={"flags": {"required": True, "client_flatten": True}} | ||||||
| ) | ||||||
| _builder.set_prop("location", AAZStrType, ".location", typ_kwargs={"flags": {"required": True}}) | ||||||
| _builder.set_prop("properties", AAZObjectType, typ_kwargs={"flags": {"client_flatten": True}}) | ||||||
| _builder.set_prop("tags", AAZDictType, ".tags") | ||||||
|
|
||||||
| properties = _builder.get(".properties") | ||||||
| if properties is not None: | ||||||
| properties.set_prop("availabilityZone", AAZStrType, ".zone", typ_kwargs={"flags": {"required": True}}) | ||||||
| properties.set_prop("provisionedBandwidthMbPerSec", AAZIntType, ".provisioned_bandwidth", typ_kwargs={"flags": {"required": True}}) | ||||||
| properties.set_prop("reservationResourceId", AAZStrType, ".reservation_id", typ_kwargs={"flags": {"required": True}}) | ||||||
| properties.set_prop("vnetInjection", AAZObjectType, ".", typ_kwargs={"flags": {"required": True}}) | ||||||
|
There was a problem hiding this comment. The second parameter "." in the vnetInjection set_prop call is likely incorrect. Following the pattern used elsewhere in the codebase (e.g., line 228 for "properties", and similar patterns in other files), when setting an object type that is composed of multiple sub-fields rather than mapping to a single argument, the second parameter should be omitted entirely. The vnetInjection object is built from subnet_id and vnet_id fields (lines 240-241), so this should probably be: Using "." as the second parameter would attempt to use the entire args context as the value, which doesn't make sense given that the actual values are set on lines 240-241.
Suggested change
|
||||||
|
|
||||||
| vnet_injection = _builder.get(".properties.vnetInjection") | ||||||
| if vnet_injection is not None: | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -13,7 +13,6 @@ | |||||
|
|
||||||
| @register_command( | ||||||
| "pscloud pool get-avs-status", | ||||||
| ) | ||||||
| class GetAvsStatus(AAZCommand): | ||||||
| """Returns the status of the storage pool connection to AVS | ||||||
|
|
@@ -49,7 +48,7 @@ def _build_arguments_schema(cls, *args, **kwargs): | |||||
| required=True, | ||||||
| ) | ||||||
| _args_schema.storage_pool_name = AAZStrArg( | ||||||
| options=["-n", "--name"], | ||||||
|
There was a problem hiding this comment. The parameter options only include For consistency and backward compatibility, this should also include
Suggested change
|
||||||
| help="Name of the storage pool", | ||||||
| required=True, | ||||||
| id_part="name", | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -13,7 +13,6 @@ | |||||
|
|
||||||
| @register_command( | ||||||
| "pscloud pool get-health-status", | ||||||
| ) | ||||||
| class GetHealthStatus(AAZCommand): | ||||||
| """Retrieve health metrics of a storage pool | ||||||
|
|
@@ -49,7 +48,7 @@ def _build_arguments_schema(cls, *args, **kwargs): | |||||
| required=True, | ||||||
| ) | ||||||
| _args_schema.storage_pool_name = AAZStrArg( | ||||||
| options=["-n", "--name"], | ||||||
|
There was a problem hiding this comment. The parameter options only include For consistency and backward compatibility, this should also include
Suggested change
|
||||||
| help="Name of the storage pool", | ||||||
| required=True, | ||||||
| id_part="name", | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,6 @@ | |
|
|
||
| @register_command( | ||
| "pscloud pool show", | ||
| ) | ||
| class Show(AAZCommand): | ||
| """Get a storage pool | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example uses the old parameter name
--storage-pool-nameinstead of the standardized--nameparameter. For consistency with the PR's goal of parameter standardization and the documented improvements on lines 12 and 154, this should be changed to use--nameor-n.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No changes made in create, changes regarding storage pool name have only been made in get-avs-status and get-health status.