You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by rafaelweingartner <gi...@git.apache.org> on 2016/03/13 16:44:39 UTC
[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...
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 <ra...@gmail.com>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...
Posted by swill <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...
Posted by swill <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...
Posted by DaanHoogland <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...
Posted by pvr9711 <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...
Posted by swill <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...
Posted by rafaelweingartner <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...
Posted by nvazquez <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...
Posted by rafaelweingartner <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...
Posted by rafaelweingartner <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...
Posted by asfgit <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fix new error found in findbugs slow buil...
Posted by rafaelweingartner <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---