[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/cloudstack/pull/1438


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1438#issuecomment-204045871
  
I opened #1427 and I agree, @rafaelweingartner and @swill so please go 
ahead. I will close #1427 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1438#issuecomment-204006448
  
I agree, but I was the one that opened the PR, so in my opinion, my opinion 
should no count much here :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1438#issuecomment-204005852
  
I think we should probably merge this one and not #1427.  Do you agree?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...

[rafaelweingartner]

Github user rafaelweingartner commented on the pull request:

https://github.com/apache/cloudstack/pull/1438#issuecomment-204001802
  
Tests were performed and reviews executed; so, should we merge this or the 
#1427?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...

[nvazquez]

Github user nvazquez commented on the pull request:

https://github.com/apache/cloudstack/pull/1438#issuecomment-203997224
  
Code LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...

[rafaelweingartner]

Github user rafaelweingartner commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1438#discussion_r57527860
  
--- Diff: 
services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
 ---
@@ -395,16 +391,9 @@ protected Answer copyFromS3ToNfs(CopyCommand cmd, 
DataTO srcData, S3TO s3, DataT
 return new CopyCmdAnswer(errMsg);
 }
 }
-
 File destFile = new File(downloadDirectory, 
substringAfterLast(srcData.getPath(), S3Utils.SEPARATOR));
-
 S3Utils.getFile(s3, s3.getBucketName(), srcData.getPath(), 
destFile).waitForCompletion();
 
-
-if (destFile == null) {
--- End diff --

I removed that because the new File (...) will always create an object that 
is referenced by destFile; therefore, it will never be == null.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1438#issuecomment-201476824
  
I don't think these tests failing are relevant to this change.  We agree 
this is ready to be merged?  Only against master?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...

[swill]

Github user swill commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1438#discussion_r57485802
  
--- Diff: 
services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
 ---
@@ -395,16 +391,9 @@ protected Answer copyFromS3ToNfs(CopyCommand cmd, 
DataTO srcData, S3TO s3, DataT
 return new CopyCmdAnswer(errMsg);
 }
 }
-
 File destFile = new File(downloadDirectory, 
substringAfterLast(srcData.getPath(), S3Utils.SEPARATOR));
-
 S3Utils.getFile(s3, s3.getBucketName(), srcData.getPath(), 
destFile).waitForCompletion();
 
-
-if (destFile == null) {
--- End diff --

Is this because `new File(...)` will always throw an exception if the value 
would be `null`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...

[pvr9711]

Github user pvr9711 commented on the pull request:

https://github.com/apache/cloudstack/pull/1438#issuecomment-200255385
  
@swill @DaanHoogland  tested this PR - LGTM.  
All BVTs have passed with the exception of the following tests
- test_07_list_default_iso - iso didn't exist.
- test_01_test_vm_volume_snapshot - test case issue - operation is not 
allowed?

Raja


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...

[rafaelweingartner]

Github user rafaelweingartner commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1438#discussion_r55938944
  
--- Diff: 
services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
 ---
@@ -395,16 +391,9 @@ protected Answer copyFromS3ToNfs(CopyCommand cmd, 
DataTO srcData, S3TO s3, DataT
 return new CopyCmdAnswer(errMsg);
 }
 }
-
 File destFile = new File(downloadDirectory, 
substringAfterLast(srcData.getPath(), S3Utils.SEPARATOR));
-
 S3Utils.getFile(s3, s3.getBucketName(), srcData.getPath(), 
destFile).waitForCompletion();
 
-
-if (destFile == null) {
--- End diff --

I removed this because there was no way this variable could be null


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...

[rafaelweingartner]

GitHub user rafaelweingartner opened a pull request:

https://github.com/apache/cloudstack/pull/1438

Fix new error found in findbugs slow build #3455

Fix new find bug error that was introduced in PR #1361 
Report: 
http://jenkins.buildacloud.org/job/build-master-slowbuild/3455/findbugsResult/new/

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/rafaelweingartner/cloudstack 
fixFindBugsBuild3455

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1438.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1438


commit b3de01a8dc039f36b8b19578936b52df50e79dc6
Author: weingartner 
Date:   2016-03-13T13:16:47Z

Fix findbugs slow build 3455




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---