Refactor loadbal order-options #1521
Conversation
There was a problem hiding this comment.
the command slcli lb order-options works well, just the message displayed as an error looks weird.
Also, the output of the slcli lb order-options --datacenter <DC> in the worked branch is the same output as the latest slcli version shows, so looks like the table requirement mentioned in the issue, is not implemented. #1520
slcli lb order-options -d dal10works fine, but should output 2 tables, instead of 1 table with a table in each column.Table 1 titled " Prices", Table 2 titled " Subnets Private Subnets"
Testing
- ✔️ slcli loadbal order-options
- slcli loadbal order-options -d dal13
worked branch output:
slcli (SoftLayer Command-line) v5.9.6 output:
| click.secho("ERROR: Use `slcli lb order-options --datacenter <DC>` " | ||
| "to find pricing information and private subnets for that specific site.") | ||
|
|
||
| else: |
There was a problem hiding this comment.
there is a lot of logic below, maybe you can get a chance to move it and encapsulate it within the manager module.
There was a problem hiding this comment.
Most of this logic is just table formatting, so I'm ok with it not being in a manager specifically, unless we want to use it in more than just this one command. Breaking it up into a few smaller sub-functions that return Table objects would make this logic a bit more readable though.
| click.secho("ERROR: Use `slcli lb order-options --datacenter <DC>` " | ||
| "to find pricing information and private subnets for that specific site.") | ||
|
|
||
| else: |
There was a problem hiding this comment.
Most of this logic is just table formatting, so I'm ok with it not being in a manager specifically, unless we want to use it in more than just this one command. Breaking it up into a few smaller sub-functions that return Table objects would make this logic a bit more readable though.
#1520