Volume backup restore, delete. #15891
Conversation
alexander-demicev
force-pushed
the
cloud-volume-backup
branch
2 times, most recently
from
85a89d0
to
b525ab3
Compare
alexander-demicev
force-pushed
the
cloud-volume-backup
branch
from
955921b
to
71cac58
Compare
alexander-demicev
changed the title
|
Code looks good to me. |
alexander-demicev
force-pushed
the
cloud-volume-backup
branch
from
71cac58
to
7efef31
Compare
app/models/cloud_volume_backup.rb
Outdated
|
|
||
| acts_as_miq_taggable | ||
|
|
||
| belongs_to :ext_management_system, :foreign_key => :ems_id, :class_name => "ExtManagementSystem" | ||
| belongs_to :availability_zone | ||
| belongs_to :cloud_volume | ||
| has_one :cloud_tenant, :through => :cloud_volume | ||
|
|
||
| def backup_restore_queue(userid, volumeid) |
There was a problem hiding this comment.
If this method is on CloudVolumeBackup then I think just restore_queue would be a better method, backup_restore_queue is redundant.
app/models/cloud_volume_backup.rb
Outdated
| MiqTask.generic_action_with_callback(task_opts, queue_opts) | ||
| end | ||
|
|
||
| def delete_backup |
|
@alexander-demichev just some nits on the method names but looks good overall |
alexander-demicev
force-pushed
the
cloud-volume-backup
branch
2 times, most recently
from
774fcc3
to
45a8442
Compare
|
@agrare Thank you for review! Does it look better now? |
app/models/cloud_volume_backup.rb
Outdated
| end | ||
|
|
||
| def restore(volume) | ||
| raw_backup_restore(volume) |
There was a problem hiding this comment.
Can you change this to raw_restore?
app/models/cloud_volume_backup.rb
Outdated
| end | ||
|
|
||
| def delete | ||
| raw_delete_backup |
app/models/cloud_volume_backup.rb
Outdated
| raw_delete_backup | ||
| end | ||
|
|
||
| def raw_delete_backup |
| - :name: Restore Backup | ||
| :description: Restore Backup to Volume | ||
| :feature_type: control | ||
| :identifier: cloud_volume_backup_restore_to_volume |
There was a problem hiding this comment.
How about cloud_volume_backup_restore?
There was a problem hiding this comment.
@agrare identifiers should be unique and cloud_volume_backup_restore is already there
There was a problem hiding this comment.
Hm is that feature used anywhere? It sounds like exactly what this does just defined on CloudVolume but I don't see any restore methods on CloudVolume
There was a problem hiding this comment.
@agrare if you mean cloud_volume_backup_restore_to_volume, then it`s used here ManageIQ/manageiq-ui-classic#2037
There was a problem hiding this comment.
Probably, I need a better name for this feature.
There was a problem hiding this comment.
@alexander-demichev actually I was wondering if cloud_volume_backup_restore could be removed from CloudVolume and moved here?
There was a problem hiding this comment.
Alright maybe we can refactor this later to use this instead?
alexander-demicev
force-pushed
the
cloud-volume-backup
branch
from
45a8442
to
f8b8535
Compare
|
Checked commits alexander-demicev/manageiq@6ebb1ee~...f8b8535 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
@agrare Thank you! |
Add cloud volume backup delete and restore actions.