Implement volume, snapshot and thin clone - #45
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #45 +/- ##
==========================================
+ Coverage 95.26% 95.63% +0.36%
==========================================
Files 62 67 +5
Lines 3739 4052 +313
==========================================
+ Hits 3562 3875 +313
Misses 177 177 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8c28fea to
5013c15
Compare
51978a6 to
347f90a
Compare
347f90a to
81ef6dd
Compare
| :param metrics: list | ||
| :return: dict | ||
| """ | ||
| metrics = metrics or VolumeConstantsGen2.DEFAULT_STATISTICS_METRICS |
There was a problem hiding this comment.
When reviewing Tao's PR, I just realized that passing empty metrics will return all metrics, but it is fine.
There was a problem hiding this comment.
Yes, this design looks convenient.
| LOG.error(msg) | ||
| raise exceptions.PowerFlexClientException(msg) | ||
|
|
||
| return self.get(entity_id=snap_id) |
There was a problem hiding this comment.
I am thinking self.get is unnecessary for sdk side, since ansible will call get at least. If updating many times, get will be called many times.
There was a problem hiding this comment.
It looks like a reserved returned value in case the consumer need the actual result of the modification. Do you think we should return none or true instead?
There was a problem hiding this comment.
I think we don't need to return anything. If it failed, exception will be thrown; otherwise it should be considered as successful, just like an ordinary update method.
There was a problem hiding this comment.
OK, I can remove the returns
4 participants
Implement volume, snapshot and thin clone