shwstppr commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1743469075
##########
api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupScheduleCmd.java:
##########
@@ -74,9 +79,14 @@ public Long getVmId() {
@Override
public void execute() throws ResourceUnavailableException,
InsufficientCapacityException, ServerApiException,
ConcurrentOperationException, ResourceAllocationException,
NetworkRuleConflictException {
try{
- BackupSchedule schedule =
backupManager.listBackupSchedule(getVmId());
- if (schedule != null) {
- BackupScheduleResponse response =
_responseGenerator.createBackupScheduleResponse(schedule);
+ List<BackupSchedule> schedules =
backupManager.listBackupSchedule(getVmId());
+ ListResponse<BackupScheduleResponse> response = new
ListResponse<>();
+ List<BackupScheduleResponse> scheduleResponses = new ArrayList<>();
+ if (Objects.nonNull(schedules) && !schedules.isEmpty()) {
Review Comment:
CollectionUtils?
##########
engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java:
##########
@@ -82,6 +87,9 @@ public class BackupVO implements Backup {
@Column(name = "zone_id")
private long zoneId;
+ @Column(name = "backed_volumes", length = 65535)
+ protected String backedVolumes;
Review Comment:
What does this store? UUID or path? Would it have been better to use a
details or backup_volume_map table?
##########
api/src/main/java/org/apache/cloudstack/backup/Backup.java:
##########
@@ -141,5 +142,6 @@ public String toString() {
Backup.Status getStatus();
Long getSize();
Long getProtectedSize();
+ List<VolumeInfo> getBackedVolumes();
Review Comment:
minor - getBackedUpVolumes?
##########
engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupDaoImpl.java:
##########
@@ -161,7 +162,14 @@ public BackupResponse newBackupResponse(Backup backup) {
response.setSize(backup.getSize());
response.setProtectedSize(backup.getProtectedSize());
response.setStatus(backup.getStatus());
- response.setVolumes(new
Gson().toJson(vm.getBackupVolumeList().toArray(), Backup.VolumeInfo[].class));
+ // ACS 4.20: For backups taken prior this release the
backup.backed_volumes column would be empty hence use vm_instance.backup_volumes
Review Comment:
Understood the use `backup.backed_volumes` wrt my preivous comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
