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/11/12 18:14:09 UTC
[jclouds] jclouds-331 - imageChooser function backport (#202)
You can merge this Pull Request by running:
git pull https://github.com/ahgittin/jclouds feature/1.6.x/jclouds-331
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/202
-- Commit Summary --
* JCLOUDS-331 - support specifying an imageChooser function in TemplateBuilder
* JCLOUDS-331: Changed return type of TemplateBuilder.imageChooser
-- File Changes --
M apis/ec2/src/test/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImplTest.java (7)
M compute/src/main/java/org/jclouds/compute/domain/TemplateBuilder.java (14)
M compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java (54)
M compute/src/test/java/org/jclouds/compute/domain/internal/TemplateBuilderImplTest.java (155)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/202.patch
https://github.com/jclouds/jclouds/pull/202.diff
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by Andrew Phillips <no...@github.com>.
> @@ -168,6 +171,15 @@
> TemplateBuilder imageMatches(Predicate<Image> condition);
>
> /**
> + * Configure this template with a specific preference function which operates on
> + * images which match the other criteria.
> + * <p>
> + * If no function is supplied, jclouds will select one according to an internal strategy.
> + * This strategy may change from version to version.
> + */
> + TemplateBuilder imageChooser(Function<Iterable<? extends Image>,Image> imageChooser);
[minor] Missing space in `,Image`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202/files#r7621089
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by ahgittin <no...@github.com>.
> + imageNameAlt, new Function<TemplateBuilderImpl,TemplateBuilderImpl>() {
> + @Override
> + public TemplateBuilderImpl apply(TemplateBuilderImpl input) {
> + return input.imageChooser(new Function<Iterable<? extends Image>, Image>() {
> + @Override
> + public Image apply(Iterable<? extends Image> input) {
> + return Iterables.find(input, new Predicate<Image>() {
> + @Override
> + public boolean apply(Image input) {
> + return input.getName()!=null && input.getName().contains("alternate");
> + }
> + });
> + }
> + });
> + }
> + });
All I want for Christmas ... is closures. :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202/files#r7670324
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by Andrew Phillips <no...@github.com>.
> @@ -111,16 +147,12 @@ public void testLocationPredicateWhenComputeMetadataIsNotLocationBound() {
> }
>
> @SuppressWarnings("unchecked")
> - @Test
> - public void testResolveImages() {
> -
> -
> + protected void doTestResolveImages(Supplier<Set<? extends Image>> images, Image expectedBest,
> + Function<TemplateBuilderImpl, TemplateBuilderImpl> builderCustomisation) {
This new style makes the test very hard to understand...what does the `builderCustomisation` do..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202/files#r7621149
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #390](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/390/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202#issuecomment-28315059
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by Andrew Phillips <no...@github.com>.
> @@ -945,6 +975,15 @@ public TemplateBuilder imageMatches(Predicate<Image> condition) {
> * {@inheritDoc}
> */
> @Override
> + public TemplateBuilderImpl imageChooser(Function<Iterable<? extends Image>,Image> imageChooser) {
[minor] Missing space in `,Image`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202/files#r7621100
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by Andrew Phillips <no...@github.com>.
Some of these might have already been addressed as part of https://github.com/jclouds/jclouds/pull/207
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202#issuecomment-28705669
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by ahgittin <no...@github.com>.
@demobox if the whitespace warnings need attention can you give more details? i must have different settings as `git diff HEAD~1..HEAD` on this branch for me (and `HEAD~2..HEAD`) show nothing which looks odd -- certainly no red blocks but then i have no colouring at all, but no warnings or apparent whitespace glitches. same for the "Files Changed" link in the web browser on this request, i can't see anything wrong there.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202#issuecomment-28636924
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by Andrew Phillips <no...@github.com>.
> + imageNameAlt, new Function<TemplateBuilderImpl,TemplateBuilderImpl>() {
> + @Override
> + public TemplateBuilderImpl apply(TemplateBuilderImpl input) {
> + return input.imageChooser(new Function<Iterable<? extends Image>, Image>() {
> + @Override
> + public Image apply(Iterable<? extends Image> input) {
> + return Iterables.find(input, new Predicate<Image>() {
> + @Override
> + public boolean apply(Image input) {
> + return input.getName()!=null && input.getName().contains("alternate");
> + }
> + });
> + }
> + });
> + }
> + });
See comment above...this test seems almost impossible to decipher :-(
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202/files#r7621160
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by Andrew Phillips <no...@github.com>.
> what command were you running when you got the whitespace warnings?
Just looking for the red blocks in `git diff HEAD~1..HEAD` on the command line.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202#issuecomment-28635038
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by Andrew Phillips <no...@github.com>.
> is it better to handle them as a new issue?
Fair enough - committed to [1.6.x](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;h=8967fb7). There were a bunch of whitespace warnings from Git too, so if we want to address the comments in a follow-up commit it would be nice to clean those up at the same time.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202#issuecomment-28541848
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #853](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/853/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202#issuecomment-28315114
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by ahgittin <no...@github.com>.
thanks @demobox i can clean these up
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202#issuecomment-28707165
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by Andrew Phillips <no...@github.com>.
>
> @Override
> + protected String getProviderFormatId(String uniqueLabel) {
> + return "us-east-1/"+uniqueLabel;
> + }
> +
> +
[minor] Remove blank line
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202/files#r7621085
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by ahgittin <no...@github.com>.
cool. done in #207.
what command were you running when you got the whitespace warnings? happy to clean those up also.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202#issuecomment-28556865
Re: [jclouds] jclouds-331 - imageChooser function backport (#202)
Posted by ahgittin <no...@github.com>.
@demobox I agree with your comments but as this is a backport of pull requests already merged in 1.7.0 is it better to handle them as a new issue?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/202#issuecomment-28514592