You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Jonathan Cobb <no...@github.com> on 2014/04/19 22:26:00 UTC
[jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
preface -- I'm sure there is probably a better way to do this and I'm open to suggestions on changes.
the problem: some of the droplets in my account are long-running, and have image_ids that are no longer returned from DO's /images api call. So, when I'm trying to create a new droplet, it fails here with a NPE while iterating over the existing droplets (trying to find ones that might match the group name).
You can merge this Pull Request by running:
git pull https://github.com/cobbzilla/jclouds-labs master
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs/pull/58
-- Commit Summary --
* hack to not barf on old images that are no longer listed in the api
-- File Changes --
M digitalocean/src/main/java/org/jclouds/digitalocean/compute/functions/DropletToNodeMetadata.java (6)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/58.patch
https://github.com/jclouds/jclouds-labs/pull/58.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #984](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/984/) 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-labs/pull/58#issuecomment-41121672
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by Andrew Phillips <no...@github.com>.
> WDYT @demobox?
Agreed...if this is a corner case, the minimal possible change sounds sensible (how about a "Null image object", then, i.e. something like `Image.NULL`, rather than `null` with the attendant NPE risks?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-40893435
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #143](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/143/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-41213565
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by Ignasi Barrera <no...@github.com>.
The point is that the model only exposes the imageId, which is a String, and the OperatingSystem, both already marked as @Nullable in the NodeMetadata interface; the entire image object itself is not exposed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-40893665
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Ignasi Barrera <no...@github.com>.
I plan to merge the checkstyle PR tomorrow and then open a new one adding the test classes. So in a day or two everything should be in place to make it trivial to add the missing tests!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-43013594
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Andrew Phillips <no...@github.com>.
> I plan to merge the checkstyle PR tomorrow and then open a new one adding the test classes
Ah, great. Thanks, @nacx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-43033174
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by Andrew Phillips <no...@github.com>.
> @@ -91,8 +91,10 @@ public boolean apply(Location location) {
> }));
>
> Image image = images.get().get(String.valueOf(input.getImageId()));
> - builder.imageId(image.getId());
> - builder.operatingSystem(image.getOperatingSystem());
> + if (image != null) {
Please add a comment describing under which circumstances this may occur?
@nacx: I guess we should also have a JIRA issue and a unit test for this?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58/files#r11888711
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Jonathan Cobb <no...@github.com>.
@demobox I will patiently wait for some kind of exemplary test framework that's easy to understand so I can quickly add a test to it.
It'd be nice to get this PR merged in, but I'm not in a huge rush.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-43022967
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by Ignasi Barrera <no...@github.com>.
That would be cleaner but require to change the portable model (affecting all providers). For the moment I'd see what happens when leaving them `null`, and if we find this being a common/real use case in other providers (how often does this happen?) we can properly model it in the portable interface. WDYT @demobox?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-40889300
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #142](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/142/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-41121536
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #141](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/141/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-40880118
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Andrew Phillips <no...@github.com>.
> I've just opened JCLOUDS-543 to track this.
Thanks, @nacx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-41133919
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Andrew Phillips <no...@github.com>.
> I've just opened #62 adding the missing tests.
Thanks, @nacx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-43465319
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Ignasi Barrera <no...@github.com>.
@demobox @cobbzilla I've just opened https://github.com/jclouds/jclouds-labs/pull/62 adding the missing tests.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-43331423
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Andrew Phillips <no...@github.com>.
@nacx @cobbzilla Ping on this? Just wondering where we're currently at here..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-43007429
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Jonathan Cobb <no...@github.com>.
thanks @nacx -- I would much prefer adding one test to an existing test suite that clearly illustrates the patterns/conventions you'd like to see followed, versus stumbling around in the dark :)
If you do end up creating some tests, could you post to this thread and point me to the appropriate place where you'd like me to add a test for this PR?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-41235562
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Andrew Phillips <no...@github.com>.
> I'll try to have a PR for that soon if you don't beat me! :)
I'm not in the race, but perhaps @cobbzilla is ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-43519052
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Jonathan Cobb <no...@github.com>.
I added the comment. Re adding a test, I have to admit I'm pretty swamped and I may take a long while getting to it. Time permitting, I will try.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-41212305
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Ignasi Barrera <no...@github.com>.
This is also affecting the Jenkins plugin:
https://issues.jenkins-ci.org/browse/JENKINS-22963
As commented in the issue there, now that thay've addwd support for slugs everywhere I think the right fix would be to use them everywhere instead of the ids. I'll try to have a PR for that soon if you don't beat me! :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-43467753
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by Andrew Phillips <no...@github.com>.
> and the OperatingSystem, both already marked as @Nullable in the NodeMetadata interface
Oops...lazy me ;-) In that case, `null` seems indeed like a good option.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-40901206
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by Jonathan Cobb <no...@github.com>.
yup, that change seems to have worked fine -- the integration test I have that uses this code still passes A-OK.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-41121132
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by Andrew Phillips <no...@github.com>.
> if you just leave the image (and all image derived fields) to null
Would an `Optional` perhaps make sense here, just as an alternative suggestion?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-40885780
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Andrew Phillips <no...@github.com>.
> I plan to merge the checkstyle PR tomorrow and then open a new one adding the test classes
Ah, great. Thanks, @nacx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-43033160
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @cobbzilla. I'm working on a PR to add tests for the existing functionality in those transformation functions (let's fix our technical debt!) and hope to open it tomorrow. If it is ok to you, you can wait until then, and just add your test to the ones I'll create.
Anyway, feel free to create the tests on your own and update this PR! :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-41214365
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by Ignasi Barrera <no...@github.com>.
I meant to not assign the imageId and operatingSystem if the image is null. Could you try that?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-41005747
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by Jonathan Cobb <no...@github.com>.
@nacx I'm not sure I understand your question -- if I let the "image" variable be null, the very next line (`builder.imageId(image.getId());`) causes a NPE.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-40999874
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Jonathan Cobb <no...@github.com>.
Sure -- I'm fine with merging it. The only potential problem would be if some code relied on the builder having a non-null imageId or operatingSystem, in which case we've just pushed the NPE to somewhere else. But the alternative is to bomb out on a lot of stuff that would otherwise work just fine with a null imageId/operatingSystem. ceterus paribus, I think this fix is the better way.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-43586438
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #982](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/982/) 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-labs/pull/58#issuecomment-40880061
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by Ignasi Barrera <no...@github.com>.
> I guess we should also have a JIRA issue and a unit test for this?
I've just opened [JCLOUDS-543](https://issues.apache.org/jira/browse/JCLOUDS-543) to track this.
There is no unit test class for the `DropletToNodeMetadata` class. @cobbzilla, mind creating the `DropletToNodeMetadataTest` class and add a unit test that validates your change? We use `EasyMock` for this kind of unit tests. If you need some reference, you can take a look at how the [EC2](https://github.com/jclouds/jclouds/blob/master/apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/RunningInstanceToNodeMetadataTest.java) or [Abiquo](https://github.com/jclouds/jclouds-labs/tree/master/abiquo/src/test/java/org/jclouds/abiquo/compute/functions) tests work. There's no need to add a complete test for all cases in this PR, just add one to validate your changes. Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-41133688
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by Ignasi Barrera <no...@github.com>.
Thanks for the contribution @cobbzilla!
Out of curiosity, what happens if you just leave the image (and all image derived fields) to `null`? If possible, I think it makes more sense to have a node without image, to express it does no longer exist, than having the wrong one set.
It's nice to have DO users and to see use cases like this! Your long-living droplets will help us umderstand some of their api internals :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-40883910
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by Ignasi Barrera <no...@github.com>.
And also lazy me! I didn't express it properly in my first comment :) It's always good to discuss these kind of details!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-40904370
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Ignasi Barrera <no...@github.com>.
Thanks for thw ping @demobox! I started this but completely forgot! :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-43013772
Re: [jclouds-labs] hack to not barf on old images that are no longer
listed in the api (#58)
Posted by Jonathan Cobb <no...@github.com>.
@nacx I see what you mean -- yes I can try that. I'll report back after I've given it a whirl.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-41006274
Re: [jclouds-labs] JCLOUDS-543: hack to not barf on old images that
are no longer listed in the api (#58)
Posted by Ignasi Barrera <no...@github.com>.
I've just opened https://github.com/jclouds/jclouds-labs/pull/63 adding support for using the "slug" values as IDs to properly fix the entire thing, now that we know more a bit more about the DO api. It was not as straightforward as I had initially supposed (now methods have been added to the api), but I think it will make the provider easier to use.
If that PR buids without issues (I've executed the live tests locally and all pass), I think we'd better merge it and close this one. Are you ok with this @cobbzilla? Many thanks for the investigation and the pointers! That has definitely been the key point to these improvements and helped a lot identifying the Jenkins issue!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/58#issuecomment-43524689