-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[VMWARE-818] Add ability to get backup size directly for vm_restore_p… #7
[VMWARE-818] Add ability to get backup size directly for vm_restore_p… #7
Conversation
def get_backup_size(vm_restore_point_hash) | ||
refs = vm_restore_point_hash.dig(:Links, :Link) | ||
|
||
back_up_file_id = refs.detect { |hash| hash[:Type] == 'BackupFileReference' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will raise error if refs
does not contain any link with Type == "BackupFileReference
. (as refs.detect
will return nil
that does not have fetch
method) If such situation is possible it's best to check for presence after detect
and return 0 in case there is no such link
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also I suggest renaming the var to backup_file_id
to be consistent with naming (it's backup
everywhere else, not back_up
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll add safe navigation operator here
.max_by { |hash| Time.parse(hash[:creationtimeutc]) if hash[:creationtimeutc] } | ||
&.fetch(:backupsize, 0) | ||
def get_backup_size(vm_restore_point_hash) | ||
refs = vm_restore_point_hash.dig(:Links, :Link) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you sure this works? .dig
works only for hashes, and vm_restore_point_hash[:Links]
will be an array if I'm not mistaken (I've looked at https://helpcenter.veeam.com/docs/backup/em_rest/restorepoints_id_vmrestorepoints.html?ver=110 for API reference)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vm_restore_point_hash variable here has hash inside like this
{:Links=> {:Link=> [{:Href=>"https://10.63.0.92:9398/api/backupServers/bd76362f-fa63-4d9a-a988-599b41c0f423", :Name=>"vc-b-backup.onappdev.lviv", :Type=>"BackupServerReference", :Rel=>"Up"}, {:Href=>"https://10.63.0.92:9398/api/restorePoints/f1734567-c830-40bd-8476-e935449c788b", :Name=>"Apr 7 2022 11:49AM", :Type=>"RestorePointReference", :Rel=>"Up"}, {:Href=>"https://10.63.0.92:9398/api/backupFiles/bf1f4635-ead1-4f5a-ae4b-adabac3b0e75", :Name=>"Hourly 4@ms_centOS6_vmwaretoolsD2022-04-07T144924_F0CB.vib", :Type=>"BackupFileReference", :Rel=>"Up"}, {:Href=>"https://10.63.0.92:9398/api/vmRestorePoints/2e9b79cb-f1ff-4d63-8bf7-c3efdcf0a83c?format=Entity", :Name=>"ms_centOS6_vmwaretools-VNAN@2022-04-07 11:49:48", :Type=>"VmRestorePoint", :Rel=>"Alternate"}]}, :UID=>"urn:veeam:VmRestorePoint:2e9b79cb-f1ff-4d63-8bf7-c3efdcf0a83c", :Name=>"ms_centOS6_vmwaretools-VNAN@2022-04-07 11:49:48", :Href=>"https://10.63.0.92:9398/api/vmRestorePoints/2e9b79cb-f1ff-4d63-8bf7-c3efdcf0a83c", :Type=>"VmRestorePointReference"}
It will work
|
||
return 0 unless back_up_file_id | ||
|
||
backup_repository_ref = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're not using backup_repository_ref
also won't just api_get("backupFiles/#{back_up_file_id}").dig(:BackupFile, :BackupSize)
be enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
…oint