JoaoJandre commented on code in PR #6589:
URL: https://github.com/apache/cloudstack/pull/6589#discussion_r1567488377


##########
engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupDaoImpl.java:
##########
@@ -141,32 +141,37 @@ public List<Backup> syncBackups(Long zoneId, Long vmId, 
List<Backup> externalBac
 
     @Override
     public BackupResponse newBackupResponse(Backup backup) {
-        VMInstanceVO vm = 
vmInstanceDao.findByIdIncludingRemoved(backup.getVmId());
-        AccountVO account = 
accountDao.findByIdIncludingRemoved(vm.getAccountId());
-        DomainVO domain = domainDao.findByIdIncludingRemoved(vm.getDomainId());
-        DataCenterVO zone = 
dataCenterDao.findByIdIncludingRemoved(vm.getDataCenterId());
-        BackupOffering offering = 
backupOfferingDao.findByIdIncludingRemoved(vm.getBackupOfferingId());
-
-        BackupResponse response = new BackupResponse();
-        response.setId(backup.getUuid());
-        response.setVmId(vm.getUuid());
-        response.setVmName(vm.getHostName());
-        response.setExternalId(backup.getExternalId());
-        response.setType(backup.getType());
-        response.setDate(backup.getDate());
-        response.setSize(backup.getSize());
-        response.setProtectedSize(backup.getProtectedSize());
-        response.setStatus(backup.getStatus());
-        response.setVolumes(new 
Gson().toJson(vm.getBackupVolumeList().toArray(), Backup.VolumeInfo[].class));
-        response.setBackupOfferingId(offering.getUuid());
-        response.setBackupOffering(offering.getName());
-        response.setAccountId(account.getUuid());
-        response.setAccount(account.getAccountName());
-        response.setDomainId(domain.getUuid());
-        response.setDomain(domain.getName());
-        response.setZoneId(zone.getUuid());
-        response.setZone(zone.getName());
-        response.setObjectName("backup");
-        return response;
+        try {
+            VMInstanceVO vm = 
vmInstanceDao.findByIdIncludingRemoved(backup.getVmId());
+            AccountVO account = 
accountDao.findByIdIncludingRemoved(vm.getAccountId());
+            DomainVO domain = 
domainDao.findByIdIncludingRemoved(vm.getDomainId());
+            DataCenterVO zone = 
dataCenterDao.findByIdIncludingRemoved(vm.getDataCenterId());
+            BackupOffering offering = 
backupOfferingDao.findByIdIncludingRemoved(backup.getBackupOfferingId());
+
+            BackupResponse response = new BackupResponse();
+            response.setId(backup.getUuid());
+            response.setVmId(vm.getUuid());
+            response.setVmName(vm.getHostName());
+            response.setExternalId(backup.getExternalId());
+            response.setType(backup.getType());
+            response.setDate(backup.getDate());
+            response.setSize(backup.getSize());
+            response.setProtectedSize(backup.getProtectedSize());
+            response.setStatus(backup.getStatus());
+            
response.setVolumes(GSON.toJson(backup.getBackupVolumeList().toArray(), 
Backup.VolumeInfo[].class));
+            response.setBackupOfferingId(offering.getUuid());
+            response.setBackupOffering(offering.getName());
+            response.setAccountId(account.getUuid());
+            response.setAccount(account.getAccountName());
+            response.setDomainId(domain.getUuid());
+            response.setDomain(domain.getName());
+            response.setZoneId(zone.getUuid());
+            response.setZone(zone.getName());
+            response.setObjectName("backup");
+            return response;
+        } catch (Exception e) {

Review Comment:
   Do we need a Pokemon catch here? we should just list the expected exceptions 
instead.



##########
plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java:
##########
@@ -397,6 +404,30 @@ public void doInTransactionWithoutResult(TransactionStatus 
status) {
         });
     }
 
+    protected String createVolumeInfoFromVolumes(List<String> paths) {

Review Comment:
   ```suggestion
       protected String createVolumeInfoFromVolumePaths(List<String> paths) {
   ```



##########
plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java:
##########
@@ -397,6 +404,30 @@ public void doInTransactionWithoutResult(TransactionStatus 
status) {
         });
     }
 
+    protected String createVolumeInfoFromVolumes(List<String> paths) {
+        List<VolumeVO> vmVolumes = new ArrayList<>();
+        try {
+            for (String diskName : paths) {
+                VolumeVO volumeVO = volumeDao.findByPath(diskName);
+                if (volumeVO != null) {
+                    vmVolumes.add(volumeVO);
+                }
+            }
+            List<Backup.VolumeInfo> list = new ArrayList<>();
+            for (VolumeVO vol : vmVolumes) {
+                list.add(new Backup.VolumeInfo(vol.getUuid(), vol.getPath(), 
vol.getVolumeType(), vol.getSize(), vol.getDeviceId()));
+            }
+            return new Gson().toJson(list.toArray(), 
Backup.VolumeInfo[].class);
+        } catch (Exception e) {

Review Comment:
   Do we need a pokemon catch here?



-- 
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]

Reply via email to