cpoerschke commented on a change in pull request #595:
URL: https://github.com/apache/solr/pull/595#discussion_r800892888



##########
File path: 
solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java
##########
@@ -138,6 +138,15 @@ public void call(ClusterState state, ZkNodeProps message, 
NamedList<Object> resu
 
       backupMgr.writeBackupProperties(backupProperties);
 
+      // It can't be done within aggregateResults call
+      // since endTime is filled later
+      if(backupProperties != null) {
+        // It is safe to extract results here
+        @SuppressWarnings("unchecked")
+        NamedList<Object> response = (NamedList<Object>) 
results.get("response");
+        response.add("endTime", backupProperties.getEndTime());

Review comment:
       `backupProperties.getEndTime()` would return `null` if it hadn't been 
filled in later for some reason; that could be null-checked but that would add 
complexity and so this is good; yet more complicated would be for 
`backupMgr.writeBackupProperties` to somehow return properties which it added 
or updated but that would be overcomplex and so this here is good.

##########
File path: 
solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java
##########
@@ -138,6 +138,15 @@ public void call(ClusterState state, ZkNodeProps message, 
NamedList<Object> resu
 
       backupMgr.writeBackupProperties(backupProperties);
 
+      // It can't be done within aggregateResults call
+      // since endTime is filled later
+      if(backupProperties != null) {

Review comment:
       Within `aggregateResults` the backup properties are nullable but here it 
(currently) seems always present so perhaps the null-check here could be 
omitted?




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to