DaanHoogland commented on code in PR #7224:
URL: https://github.com/apache/cloudstack/pull/7224#discussion_r1113950847


##########
plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java:
##########
@@ -317,17 +317,29 @@ private boolean checkTaskStatus(final HttpResponse 
response) throws IOException
         return false;
     }
 
+    /**
+     * Checks the status of the restore session. Checked states are "Success" 
and "Failure".<br/>
+     * There is also a timeout defined in the global configuration, 
backup.plugin.veeam.restore.timeout,<br/>
+     * that is used to wait for the restore to complete before throwing a 
{@link CloudRuntimeException}.
+     */
     protected boolean checkIfRestoreSessionFinished(String type, String path) 
throws IOException {
-        for (int j = 0; j < this.restoreTimeout; j++) {
+        for (int j = 0; j < restoreTimeout; j++) {
             HttpResponse relatedResponse = get(path);
             RestoreSession session = 
parseRestoreSessionResponse(relatedResponse);
             if (session.getResult().equals("Success")) {
                 return true;
             }
+            if (session.getResult().equalsIgnoreCase("Failed")) {
+                String sessionUid = session.getUid();
+                LOG.error(String.format("Failed to restore backup [%s] of VM 
[%s] due to [%s].",
+                        sessionUid, session.getVmDisplayName(),
+                        
getRestoreVmErrorDescription(StringUtils.substringAfterLast(sessionUid, ":"))));
+                throw new CloudRuntimeException(String.format("Restore job 
[%s] failed.", sessionUid));
+            }
+            LOG.debug(String.format("Waiting %s seconds, out of a total of %s 
seconds, for the backup process to finish.", j, restoreTimeout));

Review Comment:
   this is not your doing @SadiJr , i realise, but one strange thing is that 
the var `restoreTimeout` is used as a boundary of a counter and reported here 
as a number of seconds. I realize that the sleep time is a second (1000 ms) , 
but still this is not the same.



##########
plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java:
##########
@@ -317,17 +317,29 @@ private boolean checkTaskStatus(final HttpResponse 
response) throws IOException
         return false;
     }
 
+    /**
+     * Checks the status of the restore session. Checked states are "Success" 
and "Failure".<br/>
+     * There is also a timeout defined in the global configuration, 
backup.plugin.veeam.restore.timeout,<br/>
+     * that is used to wait for the restore to complete before throwing a 
{@link CloudRuntimeException}.
+     */
     protected boolean checkIfRestoreSessionFinished(String type, String path) 
throws IOException {
-        for (int j = 0; j < this.restoreTimeout; j++) {
+        for (int j = 0; j < restoreTimeout; j++) {
             HttpResponse relatedResponse = get(path);
             RestoreSession session = 
parseRestoreSessionResponse(relatedResponse);
             if (session.getResult().equals("Success")) {
                 return true;
             }
+            if (session.getResult().equalsIgnoreCase("Failed")) {
+                String sessionUid = session.getUid();
+                LOG.error(String.format("Failed to restore backup [%s] of VM 
[%s] due to [%s].",
+                        sessionUid, session.getVmDisplayName(),
+                        
getRestoreVmErrorDescription(StringUtils.substringAfterLast(sessionUid, ":"))));
+                throw new CloudRuntimeException(String.format("Restore job 
[%s] failed.", sessionUid));
+            }
+            LOG.debug(String.format("Waiting %s seconds, out of a total of %s 
seconds, for the backup process to finish.", j, restoreTimeout));
             try {
                 Thread.sleep(1000);
             } catch (InterruptedException ignored) {
-                LOG.trace(String.format("Ignoring InterruptedException [%s] 
when waiting for restore session finishes.", ignored.getMessage()));

Review Comment:
   please leave this in (or replace with an appropriate 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]

Reply via email to