Liron Ar has posted comments on this change. Change subject: core: Add support for creating in memory tar ......................................................................
Patch Set 8: (3 comments) http://gerrit.ovirt.org/#/c/23466/8/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/archivers/tar/InMemoryTar.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/archivers/tar/InMemoryTar.java: Line 17: } Line 18: Line 19: public void addTarEntry(byte[] data, String name) { Line 20: try { Line 21: TarArchiveEntry entry = new TarArchiveEntry(name); > Is it possible to minimize that try/catch block? the try block was removed, so no longer relevant Line 22: entry.setSize(data.length); Line 23: tarArchiveOutputStream.putArchiveEntry(entry); Line 24: tarArchiveOutputStream.write(data); Line 25: tarArchiveOutputStream.closeArchiveEntry(); Line 25: tarArchiveOutputStream.closeArchiveEntry(); Line 26: } catch (IOException e) { Line 27: log.error("Error while writing entry to tar", e); Line 28: closeTar(); Line 29: throw new RuntimeException(e); > Why not throw an IOException upwards? I see no point in masking this from t because IOException isn't runtime exception, so it'll require adding it to all the method signatures along the way. but it's less relevant as i ended up using this within try with resourcesstatement. Line 30: } Line 31: } Line 32: Line 33: public void closeTar() { Line 30: } Line 31: } Line 32: Line 33: public void closeTar() { Line 34: try { > +1! I decided to change your suggestion a bit, I decided to implement this as an AutoCloseable and used it with java 7 try-with-resources rather than closeable. Line 35: tarArchiveOutputStream.close(); Line 36: } catch(IOException e) { Line 37: log.error("Error while closing tar", e); Line 38: throw new RuntimeException(e); -- To view, visit http://gerrit.ovirt.org/23466 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I090ab5f9323dca57e8de191c1bc32cd5a164b71d Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Ar <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
