You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by ahgittin <no...@github.com> on 2013/10/02 15:51:12 UTC
[jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
implemented as discussed on mailing list. jira is unavailable but i will create an issue when i can. description below.
---
I'd like to tie in to jclouds's TemplateBuilder so that I can use its filtering/matching capabilities but then use custom logic to determine which of the matching images is "best" (for my custom value of best, i.e. preferred).
The driving use case is that I want a portable way to say "any recent ubuntu or centos". Normally this is the default of course but it doesn't always do the right thing when other options are specified -- broken ubuntu "alpha" images in AWS being the worst offender (asking for 16gb RAM in us-west-1 gives back an awful ubuntu 8.04 alpha, for instance!). It would also handle use cases where someone wants to say "Ubuntu 11 or 12, or CentOS 6.x, is best. failing that, Ubuntu 10 or CentOS 5.x. (and never any alpha images!)".
I'm thinking allowing to set an `imageSorter(Ordering)`. This fits with how
the TemplateBuilderImpl currently works (it already uses an Ordering, you just can't change it; and it stays in line with the naming convention of `Sorter` as in `Ordering hardwareSorter()`.)
You can merge this Pull Request by running:
git pull https://github.com/ahgittin/jclouds feature/templatebuilder-images-ordering
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/166
-- Commit Summary --
* support specifying an imageSorter(Ordering<Image>) for the case when multiple images match
-- File Changes --
M compute/src/main/java/org/jclouds/compute/domain/TemplateBuilder.java (8)
M compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java (32)
M compute/src/test/java/org/jclouds/compute/domain/internal/TemplateBuilderImplTest.java (95)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/166.patch
https://github.com/jclouds/jclouds/pull/166.diff
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #264](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/264/) 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/166#issuecomment-25541314
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #727](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/727/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25555757
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by Andrew Phillips <no...@github.com>.
> is it easy enough to backport this to 1.6 branch
Could you open a PR with this diff against 1.6.x? Don't want to promise anything (it's up to @andrewgaul), but it might just make it in for 1.6.3...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-26711208
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #728](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/728/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25558717
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by Andrew Phillips <no...@github.com>.
+1 - looks good to me. Would be interested in some comments from more frequent TemplateBuilder users...@nacx, you had some comments on the mailing list?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25559089
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by ahgittin <no...@github.com>.
thanks @demobox . i think i've addressed these!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25557368
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by Andrew Phillips <no...@github.com>.
> @@ -58,7 +63,7 @@
> */
> @Test(groups = "unit", singleThreaded = true, testName = "TemplateBuilderImplTest")
> public class TemplateBuilderImplTest {
> -
> +
Unintentional?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166/files#r6720658
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by ahgittin <no...@github.com>.
switch to using `Function<Iterable<? extends Image>, Image>` as per @demobox 's suggestion, on the basis that:
* it is clearer what is actually required (thanks for persuading me!)
* it isn't that much more work to write
* it avoids any mathematical disasters if someone supplies an `Ordering` function which is not well-ordered (which on thinking about it should not be done if they are being consistent, but could be done by accident...)
also repair AWS tests (and rebase) and fix whitespace
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25554418
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by ahgittin <no...@github.com>.
@demobox @nacx think we can merge this?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25705334
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by Andrew Phillips <no...@github.com>.
Committed to [master](https://git-wip-us.apache.org/repos/asf?p=incubator-jclouds.git;a=commit;h=8207c53cf23d04ace95d84bd50eb875dc0ab7438)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25713300
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by ahgittin <no...@github.com>.
@demobox - mersee bowcoo!
also (and to everybody) - is it easy enough to backport this to 1.6 branch ? that would be v useful! happy to open new pull request against that branch if that's the best way, just let me know.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-26682780
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #266](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/266/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25555736
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by ahgittin <no...@github.com>.
Anything else required before this can be merged?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25603772
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #265](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/265/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25554951
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by Andrew Phillips <no...@github.com>.
> @demobox @nacx think we can merge this?
Will do as soon as I get off this conference call, if nobody beats me to it ;-) Thanks, @ahgittin!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25707455
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #734](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/734/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25643871
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #733](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/733/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25643869
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by Andrew Phillips <no...@github.com>.
> @@ -69,6 +74,24 @@ public void testMultiMax1() {
> assertEquals(TemplateBuilderImpl.multiMax(Ordering.natural(), values), ImmutableList.of("3"));
> }
>
> + public void testMultiMax2() {
> + // check with max buried in the middle
> + Iterable<String> values = ImmutableList.of("1", "3", "2", "2");
> + assertEquals(TemplateBuilderImpl.multiMax(Ordering.natural(), values), ImmutableList.of("3"));
> + }
> +
> + public void testMultiMaxNull() {
> + // we rely on checking nulls in some Orderings, so assert it also does what we expect
> + // (unfortunately can't use ImmutableList here as that doesn't allow nulls)
> + Iterable<String> values = Arrays.asList("1", "3", null, "2", "2");
> + assertEquals(TemplateBuilderImpl.multiMax(Ordering.natural().nullsLast(), values), Arrays.asList((Object)null));
> + }
> +
> + public void testMultiMaxNull1() {
`testMultiMaxNulls`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166/files#r6720681
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by Andrew Phillips <no...@github.com>.
> DEFAULT_IMAGE_ORDERING made me think the intention was to allow it to be overridden
Yes, I was curious about the name too. Wasn't sure, though, whether it was intended to be overridden by the **user** or for certain **providers**. I'm also on the fence; just think that if we go for ordering we need to document the "contract" that the template builder parses an _ordered_ list of matching images and returns the top one. Then it obviously makes sense for you to be able to influence the ordering.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25548503
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by ahgittin <no...@github.com>.
typo, marked this as JCLOUDS-331 (as that's the issue, now properly filed in jira)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25641068
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by ahgittin <no...@github.com>.
Interesting suggestion @demobox .
Can you think of a practical situation where a choice function would be preferable to an ordering?
I think most if not all choice functions would be based on scoring or pairwise comparisons. But is it *most* or *all* ? If the former then we should change. OTOH if the ordering is always the natural implementation then leaving it as is feels right, and the purist choice function gives the user more work. (The name of the internal field is called `DEFAULT_IMAGE_ORDERING` made me think the intention was to allow it to be overridden.)
I'm on the fence.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25543366
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by Andrew Phillips <no...@github.com>.
I'll repeat my comment from the mailing list ;-)
The fact that the final image returned is chosen via an ordering is currently an internal implementation detail - should we really be exposing that? What the user wants to do here really is take a collection (well, an iterable really) of possible matches and choose the best one themselves.
Would a `Function<Iterable<? extends Image>, Image>` not be a more natural definition for this "choice function" that leaves the ordering logic as an implementation detail?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25540696
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #272](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/272/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25643860
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #273](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/273/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25643861
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by Andrew Phillips <no...@github.com>.
> Anything else required before this can be merged?
Squash-n-rebase..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25604560
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #462](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/462/) UNSTABLE
Looks like there's a problem with this pull request
[(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/166#issuecomment-25543904
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by Andrew Phillips <no...@github.com>.
> + imageArchNull, Functions.<TemplateBuilderImpl>identity());
> + }
> +
> + public void testResolveImagesCustomSorterPreferringNonNull() {
> + final Ordering<Image> sorterPreferringNonNullArch = new Ordering<Image>() {
> + @Override
> + public int compare(Image left, Image right) {
> + return ComparisonChain.start()
> + .compare(left.getOperatingSystem().getArch(), right.getOperatingSystem().getArch(),
> + Ordering.<String> natural().nullsFirst())
> + .compare(left, right, TemplateBuilderImpl.DEFAULT_IMAGE_ORDERING)
> + .result();
> + }
> + };
> + Assert.assertTrue(TemplateBuilderImpl.DEFAULT_IMAGE_ORDERING.compare(image64bit, imageArchNull) < 0);
> + Assert.assertTrue(sorterPreferringNonNullArch.compare(image64bit, imageArchNull) > 0);
Static import of `Assert.assertTrue`? And add a descriptive message?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166/files#r6720719
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #465](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/465/) UNSTABLE
Looks like there's a problem with this pull request
[(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/166#issuecomment-25564886
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #729](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/729/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25559066
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by Andrew Phillips <no...@github.com>.
> jclouds » jclouds #465 UNSTABLE
Timing-related [test failure](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/org.apache.jclouds$jclouds-core/465/testReport/junit/org.jclouds.rest.functions/PresentWhenApiVersionLexicographicallyAtOrAfterSinceApiVersionTest/testCacheIsFasterWhenAnnotationPresent/) - don't think this is related to this change.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25600442
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #726](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/726/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25554973
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #725](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/725/) 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/166#issuecomment-25541328
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by Andrew Phillips <no...@github.com>.
Do we need a test that supplies an "explicit" chooser function? Right now, all the tests seem to be using `imageChooserFromOrdering`.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25555160
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #267](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/267/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25558694
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by ahgittin <no...@github.com>.
oops EC2 provides a test extension. didn't realise that, will fix.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25543495
Re: [jclouds] support specifying an imageSorter(Ordering) on
TemplateBuilder (#166)
Posted by Ignasi Barrera <no...@github.com>.
Lgtm too! The function approach is more flexible as it exposes the entire Iterable, as opposed to the Ordering one where the comparison are only one by one. This covers more use cases, so lgtm!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/166#issuecomment-25571989