You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrea Turli <no...@github.com> on 2014/04/11 10:56:48 UTC
[jclouds-labs-google] add support for rhel, suse and windows images
(#23)
As per https://developers.google.com/compute/docs/operating-systems, I've added the RHEL, SUSE and Windows support to listImages and getImage
You can merge this Pull Request by running:
git pull https://github.com/andreaturli/jclouds-labs-google feature/add-operating-systems
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-google/pull/23
-- Commit Summary --
* add support for rhel, suse and windows images
-- File Changes --
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/GoogleComputeEngineConstants.java (6)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceAdapter.java (94)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/23.patch
https://github.com/jclouds/jclouds-labs-google/pull/23.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/23
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by Andrea Turli <no...@github.com>.
> .build();
> }
>
> @Override
> public Image getImage(String id) {
> return Objects.firstNonNull(api.getImageApiForProject(userProject.get()).get(id),
> - Objects.firstNonNull(api.getImageApiForProject(DEBIAN_PROJECT).get(id),
> - api.getImageApiForProject(CENTOS_PROJECT).get(id)));
> + Objects.firstNonNull(
> + Objects.firstNonNull(
> + Objects.firstNonNull(api.getImageApiForProject(DEBIAN_PROJECT).get(id),
Thanks! will do something cleaner
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/23/files#r11529037
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by Andrew Phillips <no...@github.com>.
> return Objects.firstNonNull(api.getImageApiForProject(userProject.get()).get(id),
> - Objects.firstNonNull(api.getImageApiForProject(DEBIAN_PROJECT).get(id),
> - api.getImageApiForProject(CENTOS_PROJECT).get(id)));
> -
> + Iterables.tryFind(listImages(), new Predicate<Image>() {
> + @Override
> + public boolean apply(Image input) {
> + return input.getName().equalsIgnoreCase(id);
> + }
> But I'm happy to revert the impl if you think I'm not understanding correctly the behaviour.
Oh, you know the behaviour here better than I do! And as per your explanation, this should indeed not be that expensive, especially since things are cached.
In that case, I'm OK with this, although would also be curious to hear what e.g. @abayer, @mattstep or @wrigri have to say.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/23/files#r11597426
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by Andrew Phillips <no...@github.com>.
> return Objects.firstNonNull(api.getImageApiForProject(userProject.get()).get(id),
> - Objects.firstNonNull(api.getImageApiForProject(DEBIAN_PROJECT).get(id),
> - api.getImageApiForProject(CENTOS_PROJECT).get(id)));
> -
> + Iterables.tryFind(listImages(), new Predicate<Image>() {
> + @Override
> + public boolean apply(Image input) {
> + return input.getName().equalsIgnoreCase(id);
> + }
Is there a risk that this approach is **significantly** more expensive? The previous approach queries each ImageApiForProject once. Here, we first get _all_ APIs to list _all_ images, then search through them. Am I understanding that correctly?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/23/files#r11544354
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by Andrea Turli <no...@github.com>.
> .build();
> }
>
> @Override
> public Image getImage(String id) {
> return Objects.firstNonNull(api.getImageApiForProject(userProject.get()).get(id),
> - Objects.firstNonNull(api.getImageApiForProject(DEBIAN_PROJECT).get(id),
> - api.getImageApiForProject(CENTOS_PROJECT).get(id)));
> + Objects.firstNonNull(
> + Objects.firstNonNull(
> + Objects.firstNonNull(api.getImageApiForProject(DEBIAN_PROJECT).get(id),
> + api.getImageApiForProject(CENTOS_PROJECT).get(id)),
> + Objects.firstNonNull(api.getImageApiForProject(RHEL_PROJECT).get(id),
> + api.getImageApiForProject(SUSE_PROJECT).get(id))),
> + api.getImageApiForProject(WINDOWS_PROJECT).get(id))
Not sure because looking at https://developers.google.com/compute/docs/operating-systems you need to specify a "project" which is quite confusing here, *_PROJECT are not really project for me!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/23/files#r11529067
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by Andrea Turli <no...@github.com>.
> return Objects.firstNonNull(api.getImageApiForProject(userProject.get()).get(id),
> - Objects.firstNonNull(api.getImageApiForProject(DEBIAN_PROJECT).get(id),
> - api.getImageApiForProject(CENTOS_PROJECT).get(id)));
> -
> + Iterables.tryFind(listImages(), new Predicate<Image>() {
> + @Override
> + public boolean apply(Image input) {
> + return input.getName().equalsIgnoreCase(id);
> + }
@demobox I think it could be more expensive but my idea was to take advantage of the fact that the result of listImages gets cached and so this is quite inexpensive. But I'm happy to revert the impl if you think I'm not understanding correctly the behaviour.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/23/files#r11559174
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by Adrian Cole <no...@github.com>.
killing this as we are going to wildly refactor this codebase with auto. feel free to resubmit after!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/23#issuecomment-61285262
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by Aled Sage <no...@github.com>.
> -import com.google.common.collect.ImmutableSet;
> -import com.google.common.collect.Lists;
> -import com.google.common.primitives.Ints;
> -import com.google.common.util.concurrent.Atomics;
> -import com.google.common.util.concurrent.UncheckedTimeoutException;
> -import com.google.inject.Inject;
> +import javax.annotation.Resource;
> +import javax.inject.Named;
> +import java.net.URI;
> +import java.util.List;
> +import java.util.Map;
> +import java.util.Set;
> +import java.util.concurrent.TimeUnit;
> +import java.util.concurrent.atomic.AtomicReference;
> +
> +import static com.google.common.base.Preconditions.checkNotNull;
Can you avoid re-ordering imports please.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/23/files#r11527811
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #53](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/53/) 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-labs-google/pull/23#issuecomment-40184063
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by Andrew Phillips <no...@github.com>.
> -import com.google.common.collect.ImmutableSet;
> -import com.google.common.collect.Lists;
> -import com.google.common.primitives.Ints;
> -import com.google.common.util.concurrent.Atomics;
> -import com.google.common.util.concurrent.UncheckedTimeoutException;
> -import com.google.inject.Inject;
> +import javax.annotation.Resource;
> +import javax.inject.Named;
> +import java.net.URI;
> +import java.util.List;
> +import java.util.Map;
> +import java.util.Set;
> +import java.util.concurrent.TimeUnit;
> +import java.util.concurrent.atomic.AtomicReference;
> +
> +import static com.google.common.base.Preconditions.checkNotNull;
+1. Please keep the original import order, if possible.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/23/files#r11544026
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #823](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/823/) 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-labs-google/pull/23#issuecomment-40201226
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by Aled Sage <no...@github.com>.
> .build();
> }
>
> @Override
> public Image getImage(String id) {
> return Objects.firstNonNull(api.getImageApiForProject(userProject.get()).get(id),
> - Objects.firstNonNull(api.getImageApiForProject(DEBIAN_PROJECT).get(id),
> - api.getImageApiForProject(CENTOS_PROJECT).get(id)));
> + Objects.firstNonNull(
> + Objects.firstNonNull(
> + Objects.firstNonNull(api.getImageApiForProject(DEBIAN_PROJECT).get(id),
Shame `Objects.firstNonNull` doesn't also accept a vararg, or overloads with more than two params!
Code would feel cleaner to me if you separated out a method `private <T> T firstNonNull(T... args)` that just did something like `for (T arg : args) { if (arg != null) return arg; }; throw new NullPointerException();`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/23/files#r11527780
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by Aled Sage <no...@github.com>.
Thanks @andreaturli - this fix will certainly be really useful for us, as some customers obviously have requirements for running/testing against other distros like SLES!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/23#issuecomment-40195910
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #821](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/821/) 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-labs-google/pull/23#issuecomment-40183986
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by Aled Sage <no...@github.com>.
> .build();
> }
>
> @Override
> public Image getImage(String id) {
> return Objects.firstNonNull(api.getImageApiForProject(userProject.get()).get(id),
> - Objects.firstNonNull(api.getImageApiForProject(DEBIAN_PROJECT).get(id),
> - api.getImageApiForProject(CENTOS_PROJECT).get(id)));
> + Objects.firstNonNull(
> + Objects.firstNonNull(
> + Objects.firstNonNull(api.getImageApiForProject(DEBIAN_PROJECT).get(id),
> + api.getImageApiForProject(CENTOS_PROJECT).get(id)),
> + Objects.firstNonNull(api.getImageApiForProject(RHEL_PROJECT).get(id),
> + api.getImageApiForProject(SUSE_PROJECT).get(id))),
> + api.getImageApiForProject(WINDOWS_PROJECT).get(id))
Do we really want just those types? What if ubuntu is added to GCE? We'll get the same problem of jclouds being unusable for that use-case. Can we have a fall-back that doesn't care what OS it is?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/23/files#r11527808
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by Andrew Phillips <no...@github.com>.
> jclouds » jclouds-labs-google #823 UNSTABLE
> jclouds-labs-google-pull-requests #54 UNSTABLE
These look like [real](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/823/testReport/) [test failures](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/org.apache.jclouds.labs$google-compute-engine/54/testReport/). Could you have a look at those.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/23#issuecomment-40236306
Re: [jclouds-labs-google] add support for rhel, suse and windows
images (#23)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #54](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/54/) 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-labs-google/pull/23#issuecomment-40201360