You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Everett Toews <no...@github.com> on 2013/10/15 21:01:25 UTC
[jclouds-labs-openstack] Initial implementation of OpenStack Marconi
and Rackspace Cloud Queues. (#42)
https://issues.apache.org/jira/browse/JCLOUDS-340
Had to include both because there's no implementation of Marconi in DevStack yet and Cloud Queues is the only deployment of Marconi right now for testing.
To run the live tests do:
cd rackspace-cloudqueues-us/
mvn -Plive -Dtest.rackspace-cloudqueues-us.identity=raxUsername -Dtest.rackspace-cloudqueues-us.credential=raxApiKey clean install
You can merge this Pull Request by running:
git pull https://github.com/rackerlabs/jclouds-labs-openstack openstack-marconi-rax-cloudqueues
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-openstack/pull/42
-- Commit Summary --
* Initial implementation of OpenStack Marconi and Rackspace Cloud Queues.
-- File Changes --
M README.md (2)
A openstack-marconi/README.md (7)
A openstack-marconi/pom.xml (135)
A openstack-marconi/src/main/java/org/jclouds/openstack/marconi/v1/MarconiApi.java (50)
A openstack-marconi/src/main/java/org/jclouds/openstack/marconi/v1/MarconiApiMetadata.java (93)
A openstack-marconi/src/main/java/org/jclouds/openstack/marconi/v1/config/MarconiHttpApiModule.java (40)
A openstack-marconi/src/main/java/org/jclouds/openstack/marconi/v1/config/MarconiTypeAdapters.java (29)
A openstack-marconi/src/main/java/org/jclouds/openstack/marconi/v1/features/QueueApi.java (52)
A openstack-marconi/src/main/java/org/jclouds/openstack/marconi/v1/handlers/MarconiErrorHandler.java (48)
A openstack-marconi/src/main/resources/META-INF/services/org.jclouds.apis.ApiMetadata (18)
A openstack-marconi/src/test/java/org/jclouds/openstack/marconi/v1/features/QueueApiLiveTest.java (35)
A openstack-marconi/src/test/java/org/jclouds/openstack/marconi/v1/features/QueueApiMockTest.java (54)
A openstack-marconi/src/test/java/org/jclouds/openstack/marconi/v1/internal/BaseMarconiApiLiveTest.java (37)
A openstack-marconi/src/test/resources/logback.xml (71)
A rackspace-cloudqueues-us/README.md (7)
A rackspace-cloudqueues-us/pom.xml (164)
A rackspace-cloudqueues-us/src/main/java/org/jclouds/rackspace/cloudqueues/us/CloudQueuesUSProviderMetadata.java (115)
A rackspace-cloudqueues-us/src/main/resources/META-INF/services/org.jclouds.providers.ProviderMetadata (1)
A rackspace-cloudqueues-us/src/test/java/org/jclouds/rackspace/cloudqueues/us/CloudQueuesUSQueueApiLiveTest.java (30)
A rackspace-cloudqueues-us/src/test/resources/logback.xml (69)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-openstack/pull/42.patch
https://github.com/jclouds/jclouds-labs-openstack/pull/42.diff
Re: [jclouds-labs-openstack] Initial implementation of OpenStack
Marconi and Rackspace Cloud Queues. (#42)
Posted by Everett Toews <no...@github.com>.
> + * @author Everett Toews
> + */
> +@Test
> +public class QueueApiMockTest extends BaseOpenStackMockTest<MarconiApi> {
> +
> + public void createQueue() throws Exception {
> + MockWebServer server = mockOpenStackServer();
> + server.enqueue(new MockResponse().setBody(accessRackspace));
> + server.enqueue(new MockResponse().setResponseCode(204));
> +
> + try {
> + MarconiApi api = api(server.getUrl("/").toString(), "openstack-marconi");
> + QueueApi queueApi = api.getQueueApiForZone("DFW");
> + boolean success = queueApi.create("jclouds-test");
> +
> + assertTrue(success);
I don't see a real need for messages on these asserts. I think the code says it well enough here.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/42/files#r7065704
Re: [jclouds-labs-openstack] Initial implementation of OpenStack
Marconi and Rackspace Cloud Queues. (#42)
Posted by Everett Toews <no...@github.com>.
> +package org.jclouds.openstack.marconi.v1.features;
> +
> +import org.jclouds.openstack.marconi.v1.internal.BaseMarconiApiLiveTest;
> +import org.testng.annotations.Test;
> +
> +import static org.testng.Assert.assertTrue;
> +
> +@Test(groups = "live", testName = "QueueApiLiveTest")
> +public class QueueApiLiveTest extends BaseMarconiApiLiveTest {
> +
> + public void create() throws Exception {
> + for (String zoneId : api.getConfiguredZones()) {
> + QueueApi queueApi = api.getQueueApiForZone(zoneId);
> + boolean success = queueApi.create("jclouds-test");
> +
> + assertTrue(success);
I don't see a real need for messages on these asserts. I think the code says it well enough here.
Many of the Queue API calls just return a 204 No Content so there's really nothing else to check.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/42/files#r7065696
Re: [jclouds-labs-openstack] Initial implementation of OpenStack
Marconi and Rackspace Cloud Queues. (#42)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #79](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/79/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/42#issuecomment-26362837
Re: [jclouds-labs-openstack] Initial implementation of OpenStack
Marconi and Rackspace Cloud Queues. (#42)
Posted by Andrew Phillips <no...@github.com>.
> +package org.jclouds.openstack.marconi.v1.features;
> +
> +import org.jclouds.openstack.marconi.v1.internal.BaseMarconiApiLiveTest;
> +import org.testng.annotations.Test;
> +
> +import static org.testng.Assert.assertTrue;
> +
> +@Test(groups = "live", testName = "QueueApiLiveTest")
> +public class QueueApiLiveTest extends BaseMarconiApiLiveTest {
> +
> + public void create() throws Exception {
> + for (String zoneId : api.getConfiguredZones()) {
> + QueueApi queueApi = api.getQueueApiForZone(zoneId);
> + boolean success = queueApi.create("jclouds-test");
> +
> + assertTrue(success);
Agree that the code is clear. It's just that if this test fails in a Jenkins run, you only see the line, and no message. So you have to _go_ to the code to see what went wrong. Adding a message makes it clearer what the actual failure is.
In cases like this where there's only _one_ assert, the name of the test indeed is enough, so I'm also fine with leaving as-is. Once you have multiple asserts in a test, though, it's useful to be able to see which one failed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/42/files#r7066232
Re: [jclouds-labs-openstack] Initial implementation of OpenStack
Marconi and Rackspace Cloud Queues. (#42)
Posted by Everett Toews <no...@github.com>.
> +import org.jclouds.http.HttpResponse;
> +import org.jclouds.http.HttpResponseException;
> +import org.jclouds.rest.AuthorizationException;
> +
> +import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream;
> +
> +public class MarconiErrorHandler implements HttpErrorHandler {
> +
> + public void handleError(HttpCommand command, HttpResponse response) {
> + // it is important to always read fully and close streams
> + byte[] data = closeClientButKeepContentStream(response);
> + String message = (data != null) ? new String(data) : null;
> +
> + Exception exception = (message != null) ?
> + new HttpResponseException(command, response, message) :
> + new HttpResponseException(command, response);
This is much better. Will update in https://github.com/jclouds/jclouds-labs-openstack/pull/43
Thanks @demobox !
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/42/files#r7065785
Re: [jclouds-labs-openstack] Initial implementation of OpenStack
Marconi and Rackspace Cloud Queues. (#42)
Posted by Andrew Phillips <no...@github.com>.
Couple of minor comments, that's all. +1 - looks good to me! Thanks, @everett-toews!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/42#issuecomment-26562310
Re: [jclouds-labs-openstack] Initial implementation of OpenStack
Marconi and Rackspace Cloud Queues. (#42)
Posted by Everett Toews <no...@github.com>.
> + properties.setProperty(PROPERTY_ZONE + ".IAD." + ISO3166_CODES, "US-VA");
> + properties.setProperty(PROPERTY_ZONE + ".SYD." + ISO3166_CODES, "AU-NSW");
> + properties.setProperty(PROPERTY_ZONE + ".HKG." + ISO3166_CODES, "HK");
> + return properties;
> + }
> +
> + public static class Builder extends BaseProviderMetadata.Builder {
> +
> + protected Builder(){
> + id("rackspace-cloudqueues-us")
> + .name("Rackspace Cloud Queues US")
> + .apiMetadata(new MarconiApiMetadata().toBuilder()
> + .identityName("${userName}")
> + .credentialName("${apiKey}")
> + .defaultEndpoint("https://identity.api.rackspacecloud.com/v2.0/")
> + .endpointName("identity service url ending in /v2.0/")
They are indeed different but, you're right, that could be made more clear here. I'll update the endpointName in https://github.com/jclouds/jclouds-labs-openstack/pull/43
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/42/files#r7065747
Re: [jclouds-labs-openstack] Initial implementation of OpenStack
Marconi and Rackspace Cloud Queues. (#42)
Posted by Andrew Phillips <no...@github.com>.
> + * @author Everett Toews
> + */
> +@Test
> +public class QueueApiMockTest extends BaseOpenStackMockTest<MarconiApi> {
> +
> + public void createQueue() throws Exception {
> + MockWebServer server = mockOpenStackServer();
> + server.enqueue(new MockResponse().setBody(accessRackspace));
> + server.enqueue(new MockResponse().setResponseCode(204));
> +
> + try {
> + MarconiApi api = api(server.getUrl("/").toString(), "openstack-marconi");
> + QueueApi queueApi = api.getQueueApiForZone("DFW");
> + boolean success = queueApi.create("jclouds-test");
> +
> + assertTrue(success);
Add a message to the assert?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/42/files#r7051081
Re: [jclouds-labs-openstack] Initial implementation of OpenStack
Marconi and Rackspace Cloud Queues. (#42)
Posted by Andrew Phillips <no...@github.com>.
> +import org.jclouds.http.HttpResponse;
> +import org.jclouds.http.HttpResponseException;
> +import org.jclouds.rest.AuthorizationException;
> +
> +import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream;
> +
> +public class MarconiErrorHandler implements HttpErrorHandler {
> +
> + public void handleError(HttpCommand command, HttpResponse response) {
> + // it is important to always read fully and close streams
> + byte[] data = closeClientButKeepContentStream(response);
> + String message = (data != null) ? new String(data) : null;
> +
> + Exception exception = (message != null) ?
> + new HttpResponseException(command, response, message) :
> + new HttpResponseException(command, response);
[minor] Rather than the two hidden "ifs", how about
```
if (data == null) {
Exception exception = new HttpResponseException(command, response);
} else {
new HttpResponseException(command, response, new String(data));
}
```
?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/42/files#r7051055
Re: [jclouds-labs-openstack] Initial implementation of OpenStack
Marconi and Rackspace Cloud Queues. (#42)
Posted by Everett Toews <no...@github.com>.
Any takers on a review here? If not, I'll assume lazy consensus soon and commit the thing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/42#issuecomment-26509432
Re: [jclouds-labs-openstack] Initial implementation of OpenStack
Marconi and Rackspace Cloud Queues. (#42)
Posted by Andrew Phillips <no...@github.com>.
> +package org.jclouds.openstack.marconi.v1.features;
> +
> +import org.jclouds.openstack.marconi.v1.internal.BaseMarconiApiLiveTest;
> +import org.testng.annotations.Test;
> +
> +import static org.testng.Assert.assertTrue;
> +
> +@Test(groups = "live", testName = "QueueApiLiveTest")
> +public class QueueApiLiveTest extends BaseMarconiApiLiveTest {
> +
> + public void create() throws Exception {
> + for (String zoneId : api.getConfiguredZones()) {
> + QueueApi queueApi = api.getQueueApiForZone(zoneId);
> + boolean success = queueApi.create("jclouds-test");
> +
> + assertTrue(success);
Add message? "Expected queue creation to succeed" or so?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/42/files#r7051067
Re: [jclouds-labs-openstack] Initial implementation of OpenStack
Marconi and Rackspace Cloud Queues. (#42)
Posted by Andrew Phillips <no...@github.com>.
> + properties.setProperty(PROPERTY_ZONE + ".IAD." + ISO3166_CODES, "US-VA");
> + properties.setProperty(PROPERTY_ZONE + ".SYD." + ISO3166_CODES, "AU-NSW");
> + properties.setProperty(PROPERTY_ZONE + ".HKG." + ISO3166_CODES, "HK");
> + return properties;
> + }
> +
> + public static class Builder extends BaseProviderMetadata.Builder {
> +
> + protected Builder(){
> + id("rackspace-cloudqueues-us")
> + .name("Rackspace Cloud Queues US")
> + .apiMetadata(new MarconiApiMetadata().toBuilder()
> + .identityName("${userName}")
> + .credentialName("${apiKey}")
> + .defaultEndpoint("https://identity.api.rackspacecloud.com/v2.0/")
> + .endpointName("identity service url ending in /v2.0/")
Was called "Keystone base url ending in /v2.0/" in the API metadata...are these two ways of describing the same thing, or are they indeed different?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/42/files#r7051111
Re: [jclouds-labs-openstack] Initial implementation of OpenStack
Marconi and Rackspace Cloud Queues. (#42)
Posted by Everett Toews <no...@github.com>.
Well alright then...merged.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/42#issuecomment-26537742