You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by andreaturli <gi...@git.apache.org> on 2014/11/06 10:55:46 UTC
[GitHub] incubator-brooklyn pull request: fix Live-sanity check for BlobSto...
GitHub user andreaturli opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/304
fix Live-sanity check for BlobStoreExpiryTest
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/andreaturli/incubator-brooklyn fix/BlobStoreExpiryTest
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/304.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 #304
----
commit 315bd1ae23b6eaee94f562640605c32c92762253
Author: Andrea Turli <an...@gmail.com>
Date: 2014-11-06T09:46:22Z
fix Live-sanity check
----
---
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] incubator-brooklyn pull request: fix Live-sanity check for BlobSto...
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/304#issuecomment-62216498
ok @aledsage I've squashed the commits and addressed your comments. I think it is ready to merge
---
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] incubator-brooklyn pull request: fix Live-sanity check for BlobSto...
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/304#discussion_r19942482
--- Diff: locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreExpiryTest.java ---
@@ -151,7 +150,7 @@ protected void doTestRenewAuth(boolean applyFix) throws IOException {
try {
assertContainerFound();
Assert.fail("should fail as long as "+RetryOnRenew.class+" is not working");
- } catch (Exception e) {
+ } catch (AssertionError e) {
--- End diff --
Sorry, you are right I completely misunderstood the logic here.
Problem is that I'm not able to reproduce the error even putting the expiration time to 1 sec. More soon
---
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] incubator-brooklyn pull request: fix Live-sanity check for BlobSto...
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/304#issuecomment-62176654
I've double-checked the test with @aledsage's settings and it works fine if you remove line 150.
This means that the jclouds fix resolves the issue in SoftLayer but it could still be needed to old version of Keystone (v.1.1) installation.
I propose to disable the test as it could be a good reference if we need to target Openstack Keystone v.1.1
@aledsage @ahgittin thoughts?
---
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] incubator-brooklyn pull request: fix Live-sanity check for BlobSto...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/304#discussion_r19935879
--- Diff: locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreExpiryTest.java ---
@@ -151,7 +150,7 @@ protected void doTestRenewAuth(boolean applyFix) throws IOException {
try {
assertContainerFound();
Assert.fail("should fail as long as "+RetryOnRenew.class+" is not working");
- } catch (Exception e) {
+ } catch (AssertionError e) {
--- End diff --
This doesn't make sense to me. We do `Assert.fail()` on line 152, which is guaranteed to throw an `AssertionError`, then we catch it and log that we expected it.
I thought the intent of the catch-block was that the `assertContainerFound` should throw an exception (not a failing assertion) because the token had expired.
When I run it, I see the log message `failed, as expected: java.lang.AssertionError: should fail as long as class org.jclouds.openstack.handlers.RetryOnRenew is not working`, so the suggestion it "failed, as expected" isn't correct.
---
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] incubator-brooklyn pull request: fix Live-sanity check for BlobSto...
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/304#issuecomment-62127017
@aledsage that is surprising to me as well as the fix version is exactly 1.8.0. I will test it again double-checking the versions and report soon.
---
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] incubator-brooklyn pull request: fix Live-sanity check for BlobSto...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/304#issuecomment-62206522
Thanks @andreaturli
Disabling the test sounds good. Make sure there's a really good comment that points at the jclouds issue where it was fixed, to this PR (for discussion), and saying that the test would be applicable if pointed at a Swift endpoint that was using Keystone v1.1.
---
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] incubator-brooklyn pull request: fix Live-sanity check for BlobSto...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/304#issuecomment-62638240
Looks good; merging.
---
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] incubator-brooklyn pull request: fix Live-sanity check for BlobSto...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/304#issuecomment-62124804
@andreaturli that jclouds fix certainly sounds plausible for resolving the issue, and thus making the test irrelevant.
I'd like to understand why the test passes for you but fails for me. If we're using the same jclouds version (1.8.0) and brooklyn version (master), and targeting `softlayer:ams01` then this is surprising!
If we can conclude that the test (which asserts behaviour is broken without our workaround) is no longer valid, then we should definitely delete it.
---
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] incubator-brooklyn pull request: fix Live-sanity check for BlobSto...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/304#discussion_r19936529
--- Diff: locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreExpiryTest.java ---
@@ -151,7 +150,7 @@ protected void doTestRenewAuth(boolean applyFix) throws IOException {
try {
assertContainerFound();
Assert.fail("should fail as long as "+RetryOnRenew.class+" is not working");
- } catch (Exception e) {
+ } catch (AssertionError e) {
--- End diff --
+1 logic needs fixing
---
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] incubator-brooklyn pull request: fix Live-sanity check for BlobSto...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/304
---
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] incubator-brooklyn pull request: fix Live-sanity check for BlobSto...
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/304#discussion_r19949779
--- Diff: locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreExpiryTest.java ---
@@ -151,7 +150,7 @@ protected void doTestRenewAuth(boolean applyFix) throws IOException {
try {
assertContainerFound();
Assert.fail("should fail as long as "+RetryOnRenew.class+" is not working");
- } catch (Exception e) {
+ } catch (AssertionError e) {
--- End diff --
@aledsage @ahgittin
Looks like there is a fix on that file
https://issues.apache.org/jira/browse/JCLOUDS-589
could it be a solution also for
https://issues.apache.org/jira/browse/JCLOUDS-615
---
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] incubator-brooklyn pull request: fix Live-sanity check for BlobSto...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/304#issuecomment-62125022
I'm using:
brooklyn.location.named.softlayer-swift-ams01=jclouds:swift:https://ams01.objectstorage.softlayer.net/auth/v1.0
brooklyn.location.named.softlayer-swift-ams01.identity=<snip>
brooklyn.location.named.softlayer-swift-ams01.credential=<snip>
brooklyn.location.named.brooklyn-jclouds-objstore-test-1=named:softlayer-swift-ams01
---
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.
---