You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Chris Custine <no...@github.com> on 2014/06/12 11:25:47 UTC
[jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching throwing
UnsupportedOpe... (#403)
...rationException but call still succeeds
Fixes https://issues.apache.org/jira/browse/JCLOUDS-594
Should probably be backported to 1.7.x as well
You can merge this Pull Request by running:
git pull https://github.com/ccustine/jclouds fixes/JCLOUDS-594
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/403
-- Commit Summary --
* JCLOUDS-594: ComputeService.suspendNodesMatching throwing UnsupportedOperationException but call still succeeds
-- File Changes --
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceAdapter.java (6)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/403.patch
https://github.com/jclouds/jclouds/pull/403.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Ignasi Barrera <no...@github.com>.
Yep, you're right. I didn't read properly the diff and thought that the exception was completely removed. Lack of coffee!
The fix LGTM too. Apologies for the confusion!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45979238
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1286](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1286/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-47189182
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Christopher Dancy <no...@github.com>.
Sorry to bother you on vacation! Hope it's a good one! I'll be patient a while longer. It just appeared that this was lost in the mix of things and I wanted to send another PR.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-46722971
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Chris Custine <no...@github.com>.
@demobox Added backport PR #424
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-47476172
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Christopher Dancy <no...@github.com>.
Fix looks good to me
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45942629
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Ignasi Barrera <no...@github.com>.
Is it OK to "succeed" the call even if the operation the user has requested has not been executed? I mean, when calling `suspendNode`, one should expect the node to be suspended if nothing went wrong.
Also, do we want this behavior in all OpenStack providers?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45849777
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Chris Custine <no...@github.com>.
Ahh, I think I almost caused a "who's on first" moment. ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45960561
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Chris Custine <no...@github.com>.
@everett-toews Yeah I think the bug is the result of a bad PR merge or something (the original code dates to 5/2012)... I didn't go back to analyze it in detail but something was definitely amiss. Now all of that aside, are you guys saying that you don't follow the fix, or the original?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45938983
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Chris Custine <no...@github.com>.
I thought I saw a test scenario that covered this but I can't find it right now. I will check into it and add one if necessary.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-46046835
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Chris Custine <no...@github.com>.
@nacx The Optional on the server admin extension will make this behave on any openstack provider (providers without admin extension will fallback to throw the UnsupportedOperationException). I didn't grok the first part of what you said re: succeeding even if the call didn't execute, maybe that was based on the first part?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45928408
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Chris Custine <no...@github.com>.
@cdancy I needed to add some specific test coverage as stated above by @demobox but I am on vacation since early this week. I can probably add some test code Monday and I'm sure the guys will merge it after that.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-46722724
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Chris Custine <no...@github.com>.
I have added some tests and rebased on master. There was also some static expect test data that was wrong so I fixed that as well since it affected these tests. Sorry for the delay, I forgot to add these when I returned from vacation.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-47187098
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #879](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/879/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45849421
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Christopher Dancy <no...@github.com>.
Just to clarify: the call DOES execute, and succeeds, but immediately throws an exception afterward. When working with more than 1 vm/id the first of the set succeeds but again immediately throws exception. All other vm/id's in set are then ignored and/or not executed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45931548
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Andrew Phillips <no...@github.com>.
Erm...yes...that indeed looks like weird original code. Fix looks good to me, too...but do we perhaps need a test for this? Something like `suspendWithExtensionPresentSucceeds` and `suspendWithoutExtensionThrowsUOE`?
> jclouds-java-7-pull-requests #1350 UNSTABLE
Unrelates [test failure](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/org.apache.jclouds$jclouds-compute/1350/testReport/junit/org.jclouds.compute.callables/BlockUntilInitScriptStatusIsZeroThenReturnOutputTest/testloopUntilTrueOrThrowCancellationExceptionReturnsWhenPredicateIsTrueSecondTimeWhileNotCancelled/) here.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45971233
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1215](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1215/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45851066
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Christopher Dancy <no...@github.com>.
Just chiming in as I made the initial bug. If this could be back ported to at least 1.7.3 that would be great.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45899226
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Andrew Phillips <no...@github.com>.
Merged to [master](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;h=4d5f57a). Do we also want to backport this to 1.7.x?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-47374896
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Andrew Phillips <no...@github.com>.
> Added backport PR
Thanks, @ccustine! Having a look at that now...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-47680212
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1406](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1406/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-47189640
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Everett Toews <no...@github.com>.
@nacx I'm not following either. The original code was...bizarre. It threw an UOE no matter what. Even if the Admin Extension was supported.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45933049
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1350](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1350/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45849606
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #935](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/935/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-47188692
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Christopher Dancy <no...@github.com>.
Any idea when this is getting pulled? I've got a PR I'd like to send in that's dependent on this working ;)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-46718075
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Andrew Phillips <no...@github.com>.
> I definitely think this should be backported to 1.7.x
@ccustine: Looks like [the patch](https://issues.apache.org/jira/secure/attachment/12652861/JCLOUDS-594.patch) does not directly apply :-(
```
error: patch failed: apis/openstack-nova/src/test/java/org/jclouds/openstack/nov
a/v2_0/compute/NovaComputeServiceAdapterExpectTest.java:314
error: apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute
/NovaComputeServiceAdapterExpectTest.java: patch does not apply
error: patch failed: apis/openstack-nova/src/test/java/org/jclouds/openstack/nov
a/v2_0/predicates/ServerPredicatesMockTest.java:47
error: apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/predica
tes/ServerPredicatesMockTest.java: patch does not apply
Patch failed at 0001 JCLOUDS-594: ComputeService.suspendNodesMatching throwing U
nsupportedOperationException but call still succeeds
```
Could you open a PR against 1.7.x?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-47428075
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Chris Custine <no...@github.com>.
@demobox I definitely think this should be backported to 1.7.x if anyone has a few minutes to do it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-47414299
Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching
throwing UnsupportedOpe... (#403)
Posted by Everett Toews <no...@github.com>.
Fix looks good. I didn't quite follow what Ignasi was trying to say in his first question.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/403#issuecomment-45949451