You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by danbroudy <no...@github.com> on 2014/11/25 20:34:21 UTC
[jclouds-labs-google] Added TargetInstanceApi (#100)
adds to AggregatedListApi and adds relevant tests.
You can merge this Pull Request by running:
git pull https://github.com/GoogleCloudPlatform/jclouds-labs-google targetInstances
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-google/pull/100
-- Commit Summary --
* Added TargetInstanceApi
-- File Changes --
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/GoogleComputeEngineApi.java (6)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/TargetInstance.java (48)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/AggregatedListApi.java (51)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/TargetInstanceApi.java (129)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/options/TargetInstanceOptions.java (87)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/AggregatedListApiLiveTest.java (18)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/AggregatedListApiMockTest.java (20)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/TargetInstanceApiLiveTest.java (78)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/TargetInstanceApiMockTest.java (101)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseTargetInstanceListTest.java (60)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseTargetInstanceTest.java (51)
A google-compute-engine/src/test/resources/aggregated_target_instance_list.json (128)
A google-compute-engine/src/test/resources/aggregated_target_instance_list_empty.json (55)
A google-compute-engine/src/test/resources/target_instance_get.json (11)
A google-compute-engine/src/test/resources/target_instance_insert.json (7)
A google-compute-engine/src/test/resources/target_instance_list.json (30)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/100.patch
https://github.com/jclouds/jclouds-labs-google/pull/100.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by Adrian Cole <no...@github.com>.
> +
> +import org.jclouds.javax.annotation.Nullable;
> +import org.jclouds.json.SerializedNames;
> +
> +import com.google.auto.value.AutoValue;
> +
> +@AutoValue
> +public abstract class NewTargetInstance {
> +
> + public abstract String name();
> + @Nullable public abstract String description();
> + @Nullable public abstract String natPolicy();
> + @Nullable public abstract URI instance();
> +
> + @SerializedNames({"name", "description", "natPolicy", "instance"})
> + public static NewTargetInstance create(String name, String description, String natPolicy, URI instance){
you could consider hiding this as package private since you have a builder.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100/files#r20973670
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by Andrew Phillips <no...@github.com>.
> @@ -129,4 +130,21 @@ public void forwardingRules() {
> }
> assertEquals(count, 2);
> }
> +
> + public void targetInstances() {
> + Iterator<ListPage<TargetInstance>> pageIterator = api().targetInstances(maxResults(1));
> + // make sure that in spite of having only one result per page we get at
> + // least two results
> + int count = 0;
> + for (; count < 2 && pageIterator.hasNext();) {
> + ListPage<TargetInstance> result = pageIterator.next();
> + if (!result.isEmpty()) {
> + count++;
> + }
> + }
> + if (count < 2) {
> + throw new SkipException("Not enough target instances");
> it just didn't have enough data to say if it works or not.
OK. Thanks for clarifying!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100/files#r21002644
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by Adrian Cole <no...@github.com>.
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.jclouds.googlecomputeengine.options;
> +
> +import java.net.URI;
> +
> +public final class TargetInstanceOptions {
> +
> + private String name;
are all these things really optional?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100/files#r20964105
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by Adrian Cole <no...@github.com>.
> @@ -129,4 +130,21 @@ public void forwardingRules() {
> }
> assertEquals(count, 2);
> }
> +
> + public void targetInstances() {
> + Iterator<ListPage<TargetInstance>> pageIterator = api().targetInstances(maxResults(1));
> + // make sure that in spite of having only one result per page we get at
> + // least two results
> + int count = 0;
> + for (; count < 2 && pageIterator.hasNext();) {
> + ListPage<TargetInstance> result = pageIterator.next();
> + if (!result.isEmpty()) {
> + count++;
> + }
> + }
> + if (count < 2) {
> + throw new SkipException("Not enough target instances");
Skip. The code didn't fail, it just didn't have enough data to say if it
works or not.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100/files#r20979020
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #321](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/321/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100#issuecomment-64734294
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by Andrew Phillips <no...@github.com>.
> @@ -129,4 +130,21 @@ public void forwardingRules() {
> }
> assertEquals(count, 2);
> }
> +
> + public void targetInstances() {
> + Iterator<ListPage<TargetInstance>> pageIterator = api().targetInstances(maxResults(1));
> + // make sure that in spite of having only one result per page we get at
> + // least two results
> + int count = 0;
> + for (; count < 2 && pageIterator.hasNext();) {
> + ListPage<TargetInstance> result = pageIterator.next();
> + if (!result.isEmpty()) {
> + count++;
> + }
> + }
> + if (count < 2) {
> + throw new SkipException("Not enough target instances");
Should this be skip or a failure?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100/files#r20977945
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1794](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1794/) 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/100#issuecomment-64732078
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by Adrian Cole <no...@github.com>.
Closed #100.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100#event-199308637
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1796](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1796/) 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/100#issuecomment-64734506
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by danbroudy <no...@github.com>.
I updated this to have an AutoValue NewTargetInstance with a builder.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100#issuecomment-64732007
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #316](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/316/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100#issuecomment-64457693
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #320](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/320/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100#issuecomment-64731943
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by Adrian Cole <no...@github.com>.
> +
> + public Builder natPolicy(String natPolicy) {
> + this.natPolicy = natPolicy;
> + return this;
> + }
> +
> + public Builder instance(URI instance) {
> + this.instance = instance;
> + return this;
> + }
> +
> + public NewTargetInstance build() {
> + return NewTargetInstance.create(name, description, natPolicy, instance);
> + }
> +
> + public Builder(){
nit: public, so not needed. You will want a package private NewTargetInstance() ctor
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100/files#r20973660
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by Adrian Cole <no...@github.com>.
> + public abstract String name();
> + @Nullable public abstract String description();
> + public abstract URI zone();
> + public abstract String natPolicy();
> + @Nullable public abstract URI instance();
> + public abstract URI selfLink();
> +
> + @SerializedNames({"creationTimestamp", "name", "description", "zone",
> + "natPolicy", "instance", "selfLink"})
> + public static TargetInstance create(String creationTimestamp, String name,
> + String description, URI zone, String natPolicy, URI instance, URI selfLink){
> + return new AutoValue_TargetInstance(creationTimestamp, name, description, zone,
> + natPolicy, instance, selfLink);
> + }
> +
> + TargetInstance(){
@danbroudy you made a comment about this, right? yeah this is to ensure no-one tries to subclass this, but not be too strict for auto-value to work (ex private would break things)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100/files#r20970672
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by danbroudy <no...@github.com>.
> + public abstract String name();
> + @Nullable public abstract String description();
> + public abstract URI zone();
> + public abstract String natPolicy();
> + @Nullable public abstract URI instance();
> + public abstract URI selfLink();
> +
> + @SerializedNames({"creationTimestamp", "name", "description", "zone",
> + "natPolicy", "instance", "selfLink"})
> + public static TargetInstance create(String creationTimestamp, String name,
> + String description, URI zone, String natPolicy, URI instance, URI selfLink){
> + return new AutoValue_TargetInstance(creationTimestamp, name, description, zone,
> + natPolicy, instance, selfLink);
> + }
> +
> + TargetInstance(){
I added these lines to follow existing convention. Do we want them?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100/files#r20966014
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by Adrian Cole <no...@github.com>.
LG except not keen on propagating Options objects for create.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100#issuecomment-64711239
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by danbroudy <no...@github.com>.
> + public abstract String name();
> + @Nullable public abstract String description();
> + public abstract URI zone();
> + public abstract String natPolicy();
> + @Nullable public abstract URI instance();
> + public abstract URI selfLink();
> +
> + @SerializedNames({"creationTimestamp", "name", "description", "zone",
> + "natPolicy", "instance", "selfLink"})
> + public static TargetInstance create(String creationTimestamp, String name,
> + String description, URI zone, String natPolicy, URI instance, URI selfLink){
> + return new AutoValue_TargetInstance(creationTimestamp, name, description, zone,
> + natPolicy, instance, selfLink);
> + }
> +
> + TargetInstance(){
cool thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100/files#r20973512
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1787](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1787/) 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/100#issuecomment-64458553
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by danbroudy <no...@github.com>.
> + @Nullable public abstract String description();
> + public abstract URI zone();
> + public abstract String natPolicy();
> + @Nullable public abstract URI instance();
> + public abstract URI selfLink();
> +
> + @SerializedNames({"creationTimestamp", "name", "description", "zone",
> + "natPolicy", "instance", "selfLink"})
> + public static TargetInstance create(String creationTimestamp, String name,
> + String description, URI zone, String natPolicy, URI instance, URI selfLink){
> + return new AutoValue_TargetInstance(creationTimestamp, name, description, zone,
> + natPolicy, instance, selfLink);
> + }
> +
> + TargetInstance(){
> + }
I added these lines to follow existing convention. Do we want them?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100/files#r20966056
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by Adrian Cole <no...@github.com>.
and .. in! happy thanksgiving. I give thanks for your pull requests.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100#issuecomment-64735823
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by Adrian Cole <no...@github.com>.
one niggle then it's a go!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100#issuecomment-64732143
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by Adrian Cole <no...@github.com>.
> + * (the "License"); you may not use this file except in compliance with
> + * the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.jclouds.googlecomputeengine.options;
> +
> +import java.net.URI;
> +
> +public final class TargetInstanceOptions {
I prefer NewTargetInstance, as you can use auto-value to enforce nullability of fields. I imagine some aren't null. You'll likely want a builder.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100/files#r20964183
Re: [jclouds-labs-google] Added TargetInstanceApi (#100)
Posted by danbroudy <no...@github.com>.
Ready!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/100#issuecomment-64734295