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/25 15:25:47 UTC
[jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
https://issues.apache.org/jira/browse/JCLOUDS-550
You can merge this Pull Request by running:
git pull https://github.com/andreaturli/jclouds-labs-google fix/machineType-obsolete
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-google/pull/24
-- Commit Summary --
* [JCLOUDS-550] fix for obsolete machineTypes
-- File Changes --
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceAdapter.java (17)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/MachineType.java (35)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceLiveTest.java (21)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/24.patch
https://github.com/jclouds/jclouds-labs-google/pull/24.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrew Phillips <no...@github.com>.
Missing unit tests for the new functionality?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-41506919
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #872](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/872/) 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-google/pull/24#issuecomment-42644421
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by asfbot <no...@github.com>.
[jclouds-labs-google-pull-request-test #2](https://builds.apache.org/job/jclouds-labs-google-pull-request-test/2/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-45386762
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #847](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/847/) 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-google/pull/24#issuecomment-41392253
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrew Bayer <no...@github.com>.
Reopened #24.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#event-128980387
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #907](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/907/) 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-google/pull/24#issuecomment-43743771
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Ignasi Barrera <no...@github.com>.
The changes LGTM now. Just one minor thing @andreaturli. Can you remove the wild card import, please?
@demobox @mikolajz If you don't have any other comments, I'll merge this as soon as the wildcard imports are removed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-42644948
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrea Turli <no...@github.com>.
Thanks @nacx, your first comment is a very good spot: maybe better to remove the checkNotNull, ok?
For you second comment: would it make sense to add a tag to the hardware to store the deprecated status, so that one can filter on it?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-41397216
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrew Phillips <no...@github.com>.
> @@ -36,6 +42,21 @@ public GoogleComputeEngineServiceLiveTest() {
> provider = "google-compute-engine";
> }
>
> + @Override
> + protected Properties setupProperties() {
> + Properties props = super.setupProperties();
> + setCredentialFromPemFile(props, provider + ".credential");
> + return props;
> + }
> +
> + @Test
> + public void testGetImage() throws Exception {
> + Set<? extends Image> images = client.listImages();
> + String id = Iterables.getLast(images, null).getId();
> + Image image = client.getImage(id);
> + assertEquals(image.getName(), id);
`getName() == id` here? Is that really what's meant?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24/files#r12030958
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Ignasi Barrera <no...@github.com>.
I assume we can merge this PR. @andreaturli mind squashing and rebasing the commits?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-49037302
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrew Phillips <no...@github.com>.
Are we missing/do we need some unit tests around the null-to-Optional conversion for `deprecated`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-42723307
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrew Phillips <no...@github.com>.
> https://builds.apache.org/job/jclouds-labs-google-pull-request-test/
This one from you, @abayer? ;-) Anything we need to do to request "configure" rights for that job?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-45427194
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Ignasi Barrera <no...@github.com>.
A couple comments:
* When you transform an existing instance to a `NodeMetadata` object, you force a non-null hardware: https://github.com/andreaturli/jclouds-labs-google/blob/fix/machineType-obsolete/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/InstanceInZoneToNodeMetadata.java#L101-L102 This conflicts with your change to the `listHardwareProfiles` method and will throw NPEs if old nodes created with deprecated machine types exist in the provider.
* A test should be added in the `GoogleComputeEngineServiceLiveTest` that verifies that the `listHardwareProfiles` method does not return obsolete hardwares.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-41394473
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrea Turli <no...@github.com>.
> + api.getMachineTypeApiForProject(userProject.get())
> + .listInZone(DEFAULT_ZONE_NAME)
> + .concat()
> + .filter(new Predicate<MachineType>() {
> + @Override
> + public boolean apply(MachineType input) {
> + return input.getDeprecated().isPresent();
> + }
> + })
> + .transform(new Function<MachineType, String>() {
> + @Override
> + public String apply(MachineType input) {
> + return input.getId();
> + }
> + })
> + .toSet();
Are you caring about readability here? Happy to change it
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24/files#r12884292
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Ignasi Barrera <no...@github.com>.
Thx for the quick patch, btw! :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-41394507
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #873](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/873/) 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-google/pull/24#issuecomment-42645609
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1126](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1126/) 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-google/pull/24#issuecomment-49038790
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrea Turli <no...@github.com>.
I've addressed your comments and test @mikolajz CreateServer. The example is fine, but .smallest() doesn't return f1-micro but n1-standard-1 as it is the first hardware that supports the image required. In fact MachineTypeInZoneToHardware.apply has this logic where
```
.supportsImage(input.getMachineType().getImageSpaceGb() > 0
? Predicates.<Image>alwaysTrue()
: Predicates.<Image>alwaysFalse())
```
so f1-micro is filtered out. Not sure we want to address that in this PR, but it is certainly something to keep in mind.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-43743625
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #57](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/57/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-42645670
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrew Bayer <no...@github.com>.
Reopened #24.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#event-128982654
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrew Bayer <no...@github.com>.
Ignore me reopening this and the new build comment - I'm verifying PR building at https://builds.apache.org. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-45386127
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrea Turli <no...@github.com>.
@abayer @ignasi, do you mind to have a look and merge it if fine? thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-41392055
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrew Phillips <no...@github.com>.
> + api.getMachineTypeApiForProject(userProject.get())
> + .listInZone(DEFAULT_ZONE_NAME)
> + .concat()
> + .filter(new Predicate<MachineType>() {
> + @Override
> + public boolean apply(MachineType input) {
> + return input.getDeprecated().isPresent();
> + }
> + })
> + .transform(new Function<MachineType, String>() {
> + @Override
> + public String apply(MachineType input) {
> + return input.getId();
> + }
> + })
> + .toSet();
I know this is a test, but how about something like:
```
ImmutableSet.Builder<String> deprecatedMachineTypes = ImmutableSet.builder();
for (MachineType machine : api.getMachineTypeApiForProject(userProject.get())
.listInZone(DEFAULT_ZONE_NAME).concat()) {
if (input.getDeprecated().isPresent()) {
deprecatedMachineTypes.add(input.getId());
}
}
ImmutableSet<String> deprecatedMachineTypeIds = deprecatedMachineTypes.build();
```
?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24/files#r12499844
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #97](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/97/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-49039200
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #67](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/67/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-45386516
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by mikolajz <no...@github.com>.
Sorry that this dropped from my radar and I didn't reply yet. This is
good at least in the short term - it solves the problem of machine creating
from template failing (Andrea, could you check that the CreateServer
example will work if you change .hardwareId() to .fromHardware() in the
call creating the template?).
There was the discussion dev@jclouds.org about the drawback of being
unable to query the profile of existing machines with deprecated/obsolete
profiles that I didn't reply to. I will reply there.
Mikołaj
On Fri, May 9, 2014 at 10:43 AM, Ignasi Barrera <no...@github.com>wrote:
> The changes LGTM now. Just one minor thing @andreaturli<https://github.com/andreaturli>.
> Can you remove the wild card import, please?
>
> @demobox <https://github.com/demobox> @mikolajz<https://github.com/mikolajz>If you don't have any other comments, I'll merge this as soon as the
> wildcard imports are removed.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-42644948>
> .
>
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-42860882
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #61](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/61/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-43743889
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Ignasi Barrera <no...@github.com>.
> })
> private MachineType(String id, Date creationTimestamp, URI selfLink, String name, String description,
> int guestCpus, int memoryMb, int imageSpaceGb, List<ScratchDisk> scratchDisks,
> - int maximumPersistentDisks, long maximumPersistentDisksSizeGb, String zone) {
> + int maximumPersistentDisks, long maximumPersistentDisksSizeGb, String zone,
> + Deprecated deprecated) {
Annotate the field as `@Nullable`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24/files#r11996410
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #55](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/55/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-41392496
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #964](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/964/) 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-google/pull/24#issuecomment-45386462
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrew Phillips <no...@github.com>.
> }
>
>
> public Builder fromMachineType(MachineType in) {
> return super.fromResource(in).memoryMb(in.getMemoryMb()).imageSpaceGb(in.getImageSpaceGb()).scratchDisks(in
> .getScratchDisks()).maximumPersistentDisks(in.getMaximumPersistentDisks())
> - .maximumPersistentDisksSizeGb(in.getMaximumPersistentDisksSizeGb()).zone(in
> - .getZone());
> + .maximumPersistentDisksSizeGb(in.getMaximumPersistentDisksSizeGb()).zone(in.getZone())
> + .deprecated(in.getDeprecated().orNull());
Wait...we are calling `.deprecated` on the Builder here, no? And the builder's `deprecated` method expects a non-null input? Or am I missing something here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24/files#r12499679
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Ignasi Barrera <no...@github.com>.
@andreaturli could you address the last two comments (and give a try to the example @mikolajz mentioned)?. I'd like to have this merged, at least to fix the annoying deprecated issue for now.
We can come up with a better solution after continuing the discussion in the mailing list, but it would be good to have this fixed anyway.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-43722710
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -55,20 +55,11 @@
> import org.jclouds.googlecomputeengine.compute.functions.FirewallTagNamingConvention;
> import org.jclouds.googlecomputeengine.compute.options.GoogleComputeEngineTemplateOptions;
> import org.jclouds.googlecomputeengine.config.UserProject;
> -import org.jclouds.googlecomputeengine.domain.Disk;
> -import org.jclouds.googlecomputeengine.domain.Image;
> -import org.jclouds.googlecomputeengine.domain.Instance;
> +import org.jclouds.googlecomputeengine.domain.*;
Avoid using wildcard imports
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24/files#r11996365
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrew Bayer <no...@github.com>.
Closed #24.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#event-128980382
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #68](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/68/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-45386936
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Ignasi Barrera <no...@github.com>.
> }
>
>
> public Builder fromMachineType(MachineType in) {
> return super.fromResource(in).memoryMb(in.getMemoryMb()).imageSpaceGb(in.getImageSpaceGb()).scratchDisks(in
> .getScratchDisks()).maximumPersistentDisks(in.getMaximumPersistentDisks())
> - .maximumPersistentDisksSizeGb(in.getMaximumPersistentDisksSizeGb()).zone(in
> - .getZone());
> + .maximumPersistentDisksSizeGb(in.getMaximumPersistentDisksSizeGb()).zone(in.getZone())
> + .deprecated(in.getDeprecated().orNull());
Good catch. That will always fail.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24/files#r12883951
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #56](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/56/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-42644423
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @andreaturli!
>Not sure we want to address that in this PR, but it is certainly something to keep in mind.
We'd better address that in a separate PR and open a JIRA issue, if it is something that should be changed. WDYT @mikolajz?
Unless anyone says the opposite, I'm OK to merge the PR once the commits are squashed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-43748888
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Ignasi Barrera <no...@github.com>.
>your first comment is a very good spot: maybe better to remove the checkNotNull, ok?
I'm not sure. I'd prefer to wait for some more feedback in the [discussion in mailing list](http://markmail.org/message/2mt3ijd4u7lbkwpz).
>For you second comment: would it make sense to add a tag to the hardware to store the deprecated status, so that one can filter on it?
Hmmm, could you use the provider specific api in the test to get all the deprecated templates, and then make sure they are not present when listing them using the ComputeService?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24#issuecomment-41399982
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #965](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/965/) 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-google/pull/24#issuecomment-45387146
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrea Turli <no...@github.com>.
> }
>
>
> public Builder fromMachineType(MachineType in) {
> return super.fromResource(in).memoryMb(in.getMemoryMb()).imageSpaceGb(in.getImageSpaceGb()).scratchDisks(in
> .getScratchDisks()).maximumPersistentDisks(in.getMaximumPersistentDisks())
> - .maximumPersistentDisksSizeGb(in.getMaximumPersistentDisksSizeGb()).zone(in
> - .getZone());
> + .maximumPersistentDisksSizeGb(in.getMaximumPersistentDisksSizeGb()).zone(in.getZone())
> + .deprecated(in.getDeprecated().orNull());
Thanks @demobox I think you are right, there is no need to checkNotNull in builder's deprecated method. Thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24/files#r12884228
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Andrew Phillips <no...@github.com>.
> @@ -36,6 +42,21 @@ public GoogleComputeEngineServiceLiveTest() {
> provider = "google-compute-engine";
> }
>
> + @Override
> + protected Properties setupProperties() {
> + Properties props = super.setupProperties();
> + setCredentialFromPemFile(props, provider + ".credential");
> + return props;
> + }
> +
> + @Test
> + public void testGetImage() throws Exception {
> + Set<? extends Image> images = client.listImages();
> + String id = Iterables.getLast(images, null).getId();
Won't this blow up if `null` is returned as the default value? If `null` is never expected to be returned here, state that in a comment? And why "getLast" instead of e.g. "getFirst", for instance?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24/files#r12030954
Re: [jclouds-labs-google] [JCLOUDS-550] fix for obsolete machineTypes
(#24)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -263,6 +254,12 @@ private Disk createBootDisk(Template template, String instanceName) {
> builder.addAll(api.getMachineTypeApiForProject(userProject.get())
> .listInZone(zone.getId())
> .concat()
> + .filter(new Predicate<MachineType>() {
> + @Override
> + public boolean apply(MachineType input) {
> + return input.getDeprecated().isPresent() ? false : true;
Just `return !input.getDeprecated.isPresent()`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/24/files#r11996379