Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-22 Thread Jeremy Daggett 
> +  try {
> + PoppyApi poppyApi = api(server.getUrl("/").toString(), 
> "openstack-poppy", overrides);
> + FlavorApi api = poppyApi.getFlavorApi();
> +
> + List flavors = api.list().toList();
> +
> + assertThat(server.getRequestCount()).isEqualTo(2);
> + assertAuthentication(server);
> + assertRequest(server.takeRequest(), "GET", BASE_URI + "/flavors");
> +
> + assertThat(flavors).isNotEmpty();
> +
> +  } finally {
> + server.shutdown();
> +  }
> +   }

I will add the test.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23394937

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-22 Thread Jeremy Daggett 
> +import org.jclouds.json.config.GsonModule.Iso8601DateAdapter;
> +import org.jclouds.openstack.poppy.v1.PoppyApi;
> +import org.jclouds.openstack.poppy.v1.handlers.PoppyErrorHandler;
> +import org.jclouds.rest.ConfiguresHttpApi;
> +import org.jclouds.rest.config.HttpApiModule;
> +
> +import com.google.common.base.Supplier;
> +import com.google.gson.FieldNamingPolicy;
> +import com.google.inject.TypeLiteral;
> +
> +@ConfiguresHttpApi
> +public class PoppyHttpApiModule extends HttpApiModule {
> +
> +   @Override
> +   protected void configure() {
> +  bind(new TypeLiteral>() 
> {}).annotatedWith(CDN.class).to(new TypeLiteral>() {});

Sure! I followed the same pattern used in the [Cloud DNS 
API](https://github.com/jclouds/jclouds/blob/master/apis/rackspace-clouddns/src/main/java/org/jclouds/rackspace/clouddns/v1/config/CloudDNSHttpApiModule.java#L38).
 It uses the Keystone 
[`ProviderModule`](https://github.com/jclouds/jclouds/blob/master/apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/config/KeystoneAuthenticationModule.java#L90)
 because this is a global, regionless service.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23394909

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-22 Thread Jeremy Daggett 
> +
> +/**
> + * Provides access to Flavor features.
> + */
> +@Beta
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Endpoint(CDN.class)
> +@Path("/flavors")
> +public interface FlavorApi {
> +
> +   @Named("flavor:list")
> +   @GET
> +   @SelectJson("flavors")
> +   @Fallback(EmptyFluentIterableOnNotFoundOr404.class)
> +   FluentIterable list();

Great! I completely agree with using `List` in all of the APIs going 
forward.  :+1: 

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23394809

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-22 Thread Jeremy Daggett 
> +   @Override
> +   public Builder toBuilder() {
> +  return new Builder().fromApiMetadata(this);
> +   }
> +
> +   public PoppyApiMetadata() {
> +  this(new Builder());
> +   }
> +
> +   protected PoppyApiMetadata(Builder builder) {
> +  super(builder);
> +   }
> +
> +   public static Properties defaultProperties() {
> +  Properties properties = BaseHttpApiMetadata.defaultProperties();
> +  properties.setProperty(SERVICE_TYPE, "cdn");

There was a dependency on [PR 650] 
(https://github.com/jclouds/jclouds/pull/650) that I submitted yesterday and 
will be changed in the next PR for the Poppy `ServiceApi`. 

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23389705

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-22 Thread Ignasi Barrera 
Add also the tests for the `PoppyFallbacks.FalseOn500or503`.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176#issuecomment-70996598

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-22 Thread Ignasi Barrera 
> +  try {
> + PoppyApi poppyApi = api(server.getUrl("/").toString(), 
> "openstack-poppy", overrides);
> + FlavorApi api = poppyApi.getFlavorApi();
> +
> + List flavors = api.list().toList();
> +
> + assertThat(server.getRequestCount()).isEqualTo(2);
> + assertAuthentication(server);
> + assertRequest(server.takeRequest(), "GET", BASE_URI + "/flavors");
> +
> + assertThat(flavors).isNotEmpty();
> +
> +  } finally {
> + server.shutdown();
> +  }
> +   }

Add a test to verify the list fallback too.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23364932

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-22 Thread Ignasi Barrera 
> +
> +/**
> + * Provides access to Flavor features.
> + */
> +@Beta
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Endpoint(CDN.class)
> +@Path("/flavors")
> +public interface FlavorApi {
> +
> +   @Named("flavor:list")
> +   @GET
> +   @SelectJson("flavors")
> +   @Fallback(EmptyFluentIterableOnNotFoundOr404.class)
> +   FluentIterable list();

If there is no pagination, just return a regular `List`, as there is no reason 
to return such an iterable here (otherwise, implement the other methods to 
properly implement the pagination). Let's try to stop proliferating the Guava 
types in our interfaces; that will make our lives much easier in the future.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23364875

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-22 Thread Ignasi Barrera 
> +   @Override
> +   public Builder toBuilder() {
> +  return new Builder().fromApiMetadata(this);
> +   }
> +
> +   public PoppyApiMetadata() {
> +  this(new Builder());
> +   }
> +
> +   protected PoppyApiMetadata(Builder builder) {
> +  super(builder);
> +   }
> +
> +   public static Properties defaultProperties() {
> +  Properties properties = BaseHttpApiMetadata.defaultProperties();
> +  properties.setProperty(SERVICE_TYPE, "cdn");

Use the `ServiceType.CDN` constant here?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23364732

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-22 Thread Ignasi Barrera 
> +import org.jclouds.json.config.GsonModule.Iso8601DateAdapter;
> +import org.jclouds.openstack.poppy.v1.PoppyApi;
> +import org.jclouds.openstack.poppy.v1.handlers.PoppyErrorHandler;
> +import org.jclouds.rest.ConfiguresHttpApi;
> +import org.jclouds.rest.config.HttpApiModule;
> +
> +import com.google.common.base.Supplier;
> +import com.google.gson.FieldNamingPolicy;
> +import com.google.inject.TypeLiteral;
> +
> +@ConfiguresHttpApi
> +public class PoppyHttpApiModule extends HttpApiModule {
> +
> +   @Override
> +   protected void configure() {
> +  bind(new TypeLiteral>() 
> {}).annotatedWith(CDN.class).to(new TypeLiteral>() {});

I'm a bit confused here. Can you explain how this binding work, and what is it 
supposed to do?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23364778

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-21 Thread Jeremy Daggett 
Squashed and pushed to **master** a006915

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176#issuecomment-70972868

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-21 Thread Jeremy Daggett 
Closed #176.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176#event-222812036

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-21 Thread Jeremy Daggett 
@zack-shoylev I just submitted [PR 
650](https://github.com/jclouds/jclouds/pull/650) a few minutes ago to add the 
CDN service type to Keystone.

Regarding the AutoValue naming, I sent a 
[message](http://markmail.org/message/dhlr44ux74vajjtx) to the list for 
community feedback. I propose that we use `getXXX()` across the project. 

Now that the 500 error has been fixed, I propose that we squash these commits 
and merge it. The Service API will be in the next Poppy PR. WDYT?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176#issuecomment-70879202

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-21 Thread Zack Shoylev 
+1

Let's merge this so work on this can continue!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176#issuecomment-70895336

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-19 Thread Zack Shoylev 
> +
> + assertThat(online).isTrue();
> +  } finally {
> + server.shutdown();
> +  }
> +   }
> +
> +   @Test(enabled = false)
> +   public void testPingFailOn500() throws Exception {
> +  MockWebServer server = mockOpenStackServer();
> +  server.enqueue(addCommonHeaders(new 
> MockResponse().setBody(stringFromResource("/access.json";
> +  server.enqueue(addCommonHeaders(new 
> MockResponse().setResponseCode(500)));
> +
> +  try {
> + PoppyApi poppyApi = api(server.getUrl("/").toString(), 
> "openstack-poppy", overrides);
> + boolean online = poppyApi.ping();

Not completely convinced it's MWS's fault, but not done debugigng it yet.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23191718

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-19 Thread Jeremy Daggett 
> + * 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.openstack.poppy.v1.domain;
> +
> +import java.util.List;
> +import java.util.Set;
> +
> +import org.jclouds.json.SerializedNames;
> +import org.jclouds.openstack.v2_0.domain.Link;
> +
> +import com.google.auto.value.AutoValue;
> +
> +/**
> + * Representation of an OpenStack Poppy CDN Flavor.

Thanks, I plan to update the javadocs across the board once the `ServiceApi` is 
implemented and complete. HTH

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23182132

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-19 Thread Jeremy Daggett 
> + * 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.openstack.poppy.v1;
> +
> +import org.jclouds.View;
> +import org.jclouds.apis.internal.BaseApiMetadataTest;
> +import org.testng.annotations.Test;
> +
> +import com.google.common.collect.ImmutableSet;
> +import com.google.common.reflect.TypeToken;
> +
> +/**
> + * Tests the Heat {@link ApiMetadata}.

Result of copy/paste.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23182044

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-19 Thread Jeremy Daggett 
> +]
> +}
> +],
> +"token": {
> +"tenant": {
> +"name": "jclouds",
> +"id": "123123",
> +"enabled": true,
> +"description": null
> +},
> +"id": "TOKEN",
> +"expires": "2014-04-28T22:48:24Z",
> +"issued_at": "2014-04-28T21:48:24.972896"
> +}
> +}
> +}

Yes, I have been including newlines for all JSON resources.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23181991

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-19 Thread Jeremy Daggett 
> +
> + assertThat(online).isTrue();
> +  } finally {
> + server.shutdown();
> +  }
> +   }
> +
> +   @Test(enabled = false)
> +   public void testPingFailOn500() throws Exception {
> +  MockWebServer server = mockOpenStackServer();
> +  server.enqueue(addCommonHeaders(new 
> MockResponse().setBody(stringFromResource("/access.json";
> +  server.enqueue(addCommonHeaders(new 
> MockResponse().setResponseCode(500)));
> +
> +  try {
> + PoppyApi poppyApi = api(server.getUrl("/").toString(), 
> "openstack-poppy", overrides);
> + boolean online = poppyApi.ping();

Yes, it hangs on the `ping()`. Note that the test is currently disabled.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23181622

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-19 Thread Zack Shoylev 
> +
> + assertThat(online).isTrue();
> +  } finally {
> + server.shutdown();
> +  }
> +   }
> +
> +   @Test(enabled = false)
> +   public void testPingFailOn500() throws Exception {
> +  MockWebServer server = mockOpenStackServer();
> +  server.enqueue(addCommonHeaders(new 
> MockResponse().setBody(stringFromResource("/access.json";
> +  server.enqueue(addCommonHeaders(new 
> MockResponse().setResponseCode(500)));
> +
> +  try {
> + PoppyApi poppyApi = api(server.getUrl("/").toString(), 
> "openstack-poppy", overrides);
> + boolean online = poppyApi.ping();

This is hanging then?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23181493

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-19 Thread Zack Shoylev 
Looks clean. What about: "cdn" service type in jclouds and AutoValue 
nomenclature? links() vs getLinks()

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176#issuecomment-70547636

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-19 Thread Zack Shoylev 
> +]
> +}
> +],
> +"token": {
> +"tenant": {
> +"name": "jclouds",
> +"id": "123123",
> +"enabled": true,
> +"description": null
> +},
> +"id": "TOKEN",
> +"expires": "2014-04-28T22:48:24Z",
> +"issued_at": "2014-04-28T21:48:24.972896"
> +}
> +}
> +}

Did we want line endings for the json files or not?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23181319

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-19 Thread Zack Shoylev 
> + * 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.openstack.poppy.v1;
> +
> +import org.jclouds.View;
> +import org.jclouds.apis.internal.BaseApiMetadataTest;
> +import org.testng.annotations.Test;
> +
> +import com.google.common.collect.ImmutableSet;
> +import com.google.common.reflect.TypeToken;
> +
> +/**
> + * Tests the Heat {@link ApiMetadata}.

Heat?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23181211

Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy (#176)

2015-01-19 Thread Zack Shoylev 
> + * 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.openstack.poppy.v1.domain;
> +
> +import java.util.List;
> +import java.util.Set;
> +
> +import org.jclouds.json.SerializedNames;
> +import org.jclouds.openstack.v2_0.domain.Link;
> +
> +import com.google.auto.value.AutoValue;
> +
> +/**
> + * Representation of an OpenStack Poppy CDN Flavor.

Some more javadoc for the domain classes.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/176/files#r23181125