get used_size of a backup from simplyblock API#547
Conversation
a794255 to
44244d9
Compare
mxsrc
left a comment
There was a problem hiding this comment.
Generally this looks good, as usual I'm hesitant to write things to the database. This doesn't look like it's a high bandwidth thing so I'd prefer simply querying the API until we get issues with that.
Beyond generally that general thought I'm not sure about the semantics of used_size on snapshots, is that constant? If two snapshots contain a specific change and one of them get's deleted, wouldn't the used_size of the other increase?
src/api/backup_snapshots.py
Outdated
| async def _snapshot_used_size(content_name: str | None) -> int: | ||
| if content_name is None: | ||
| raise ValueError("vollume snapshot content_name empty") |
There was a problem hiding this comment.
Why do we allow for content_name to be None only to immediately raise if that's the case?
There was a problem hiding this comment.
the was done to deal with type checker.
content_name = status.get("boundVolumeSnapshotContentName")
size_bytes = quantity_to_bytes(status.get("restoreSize"))
try:
used_size_bytes = await _snapshot_used_size(content_name)
the content_name that is passed as input is possible to be both "str" and "None". That's why have allowed both and parameter.
happy to accept any alternative sugestions.
There was a problem hiding this comment.
This sanitization should happen on the caller-side, this is not a failure of the function itself.
|
The reason to store this in Database is: This PR partially solves, simplyblock/vela#195 As a part of branch list API, we return If the details are stored in database the computation of sum of all snapshots of a branch becomes easier and less costly. |
|
According to the discussion with Michael we need to take more care when storing the used resources in the database. Bascially, at any point where snapshots are deleted, the values would have to be updated. |
ddcdeee to
f184778
Compare
f184778 to
85fed04
Compare
partially fixes: simplyblock/vela#195
currently we
size_bytesof a Backup from CSI VolumeSnapshot'sRESTORESIZEfield. But there is not field to get the used size. So as a part of this PR, we get the used size by calling Simplyblock's storage API