[GitHub] rhtyd commented on a change in pull request #2224: CLOUDSTACK-10032 : Database entries for templates created from snapshots disappear after management-server service restart
rhtyd commented on a change in pull request #2224: CLOUDSTACK-10032 : Database entries for templates created from snapshots disappear after management-server service restart URL: https://github.com/apache/cloudstack/pull/2224#discussion_r131613684 ## File path: services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java ## @@ -525,6 +525,8 @@ protected Answer copySnapshotToTemplateFromNfsToNfs(CopyCommand cmd, SnapshotObj bufferWriter.write("\n"); long size = _storage.getSize(destFileFullPath); bufferWriter.write("size=" + size); +bufferWriter.close(); Review comment: @niteshsarda I see what you're saying, however I disagree. You can refactor the code in either multiple try-with-resource blocks or methods in such blocks, save the `template.properties` file first and use the processor/template-location in another block. FWIW, it's still acceptable. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on a change in pull request #2224: CLOUDSTACK-10032 : Database entries for templates created from snapshots disappear after management-server service restart
rhtyd commented on a change in pull request #2224: CLOUDSTACK-10032 : Database entries for templates created from snapshots disappear after management-server service restart URL: https://github.com/apache/cloudstack/pull/2224#discussion_r131460642 ## File path: services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java ## @@ -525,6 +525,8 @@ protected Answer copySnapshotToTemplateFromNfsToNfs(CopyCommand cmd, SnapshotObj bufferWriter.write("\n"); long size = _storage.getSize(destFileFullPath); bufferWriter.write("size=" + size); +bufferWriter.close(); Review comment: LGTM, however, should we consider a `try-with-resource` syntax that can handle proper cleanup of resources in case of errors and valid closures. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services