#959 added warning message when ordering legacy storage offerings #969
Conversation
…rings from the cli, removed restriction on IOPS being multiples of 100 for performance volumes
…idth because 80 chars is way too smal
There was a problem hiding this comment.
I like these changes overall. :)
I did leave a few comments relating to consistency between the Block and File versions of the order.py files. Also, it looks like the Travis tests are failing at the moment, but if that is being caused by something unrelated, then no worries for this particular PR.
SoftLayer/CLI/file/order.py
Outdated
| if storage_type == 'performance': | ||
| if iops is None: | ||
| raise exceptions.CLIAbort( | ||
| 'Option --iops required with Performance') |
There was a problem hiding this comment.
This is a nit-pick detail, but I did notice the similar portion in SoftLayer/CLI/block/order.py (line 84) was modified so that only one line was used for this exception.
| ) | ||
|
|
||
| if service_offering == 'performance' and snapshot_size is not None: | ||
| raise exceptions.CLIAbort( |
There was a problem hiding this comment.
As with my comment on line 74, I don't think this is a major issue, but I noticed the corresponding exception in SoftLayer/CLI/block/order.py (lines 88 and 89) was updated. If possible, it might be nice to update this one as well for consistency.
| help='The service offering package to use for placing ' | ||
| 'the order [optional, default is \'storage_as_a_service\']', | ||
| help="""The service offering package to use for placing the order. | ||
| [optional, default is \'storage_as_a_service\']. enterprise and performance are depreciated""", |
There was a problem hiding this comment.
I think this looks good, but I noticed it is only here in the block volume order command. Would it be possible to add it to SoftLayer/CLI/file/order.py as well?
added warning message when ordering legacy storage offerings from the cli, removed restriction on IOPS being multiples of 100 for performance volumes