Remove redundant validation from duplicate ordering (covered by API)#894
Remove redundant validation from duplicate ordering (covered by API)#894allmightyspiff merged 2 commits intosoftlayer:masterfrom dpickle2:feature/duplicate-order-logic-updates
Conversation
** Also add deepcopy for test fixtures to clean up some unit tests
|
Coverage decreased, but I think this is because there is less code to test now, since coverage for all three related Manager files is still at 100%. I'm not sure how to fix this one without adding unit tests elsewhere to unrelated code. I'll try to figure out what to do to address this. |
allmightyspiff
left a comment
allmightyspiff
left a comment
There was a problem hiding this comment.
if the storage class has 100% coverage then coverage going down by .06% isn't a big deal. So don't stress about making tests for other classes.
SoftLayer/managers/storage_utils.py
Outdated
| iops = _validate_dupl_performance_iops( | ||
| origin_volume, iops, duplicate_size) | ||
| if iops is None: | ||
| if isinstance(utils.lookup(origin_volume, 'provisionedIops'), str): |
There was a problem hiding this comment.
I think this check is a little unneeded. What if the API changes provisionedIops to be an int in the future?
I think just doing iops = int(origin_volume.get('provisionedIops', 0)) should be sufficient. Then you can make sure that value is > 0 and then throw your exception.
There was a problem hiding this comment.
I can do that instead, no problem.
3 participants
With the upcoming introduction of volume modification, some of the existing size-validation logic for ordering duplicate volumes is no longer valid in all cases. In this branch, I have removed most of the size and performance validation logic for ordering duplicate volumes, since this validation is done within the API anyway. While increased reliance on the API does limit how specific we can be with the SLCLI error messages, it does seem like reducing redundant validation logic in the SLCLI will help increase the SLCLI's maintainability overall. Please let me know if I have removed too much, and I will be happy to add logic back in if needed.
This branch also cleans up some unit tests by using deepcopy for copying test fixtures. This will prevent an undesirable "cascading test failure" effect that came with my previous implementation of some unit tests.