DaanHoogland commented on code in PR #13977:
URL: https://github.com/apache/cloudstack/pull/13977#discussion_r3892796900
##########
vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareContext.java:
##########
@@ -387,25 +388,35 @@ public void uploadFile(String httpMethod, String
urlString, String localFileName
OutputStream out = null;
InputStream in = null;
BufferedReader br = null;
+ long bytesWritten = 0;
try {
- out = conn.getOutputStream();
- in = new FileInputStream(localFileName);
- byte[] buf = new byte[ChunkSize];
- int len = 0;
- while ((len = in.read(buf)) > 0) {
- out.write(buf, 0, len);
+ try {
Review Comment:
I would apply the sonarCloud advice here (try-with-resources) but also
factor out the method so no try block are nested within a single method
##########
vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareContext.java:
##########
@@ -444,33 +455,88 @@ public void uploadVmdkFile(String httpMethod, String
urlString, String localFile
BufferedOutputStream bos = null;
BufferedInputStream is = null;
+ long bytesWrittenThisCall = 0;
try {
- bos = new BufferedOutputStream(conn.getOutputStream());
- is = new BufferedInputStream(new FileInputStream(localFileName));
- int bufferSize = ChunkSize;
- byte[] buffer = new byte[bufferSize];
- while (true) {
- int bytesRead = is.read(buffer, 0, bufferSize);
- if (bytesRead == -1) {
- break;
+ try {
+ bos = new BufferedOutputStream(conn.getOutputStream());
+ is = new BufferedInputStream(new
FileInputStream(localFileName));
+ int bufferSize = ChunkSize;
+ byte[] buffer = new byte[bufferSize];
+ while (true) {
+ int bytesRead = is.read(buffer, 0, bufferSize);
+ if (bytesRead == -1) {
+ break;
+ }
+ bos.write(buffer, 0, bytesRead);
+ totalBytesUpdated += bytesRead;
+ bytesWrittenThisCall += bytesRead;
+ bos.flush();
+ if (progressUpdater != null)
+ progressUpdater.action(new Long(totalBytesUpdated));
}
- bos.write(buffer, 0, bytesRead);
- totalBytesUpdated += bytesRead;
bos.flush();
- if (progressUpdater != null)
- progressUpdater.action(new Long(totalBytesUpdated));
+ } catch (IOException e) {
+ throw new IOException(String.format("Upload of %s to %s %s
failed after writing %d of %d bytes for this file " +
+ "(%d bytes total written so far for this
import): %s",
+ localFileName, httpMethod, urlString,
bytesWrittenThisCall, new File(localFileName).length(), totalBytesUpdated,
e.getMessage()), e);
+ } finally {
+ if (is != null)
+ is.close();
+ if (bos != null)
+ bos.close();
}
Review Comment:
same here, let’s not have nested try blocks in a single method.
--
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]
