You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Zack Shoylev <no...@github.com> on 2015/06/13 01:01:15 UTC
[jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Also Rackspace providers.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs-openstack/pull/193
-- Commit Summary --
* Additional support for heat features. Autovalue refactorings. Rax providers.
-- File Changes --
M openstack-heat/pom.xml (2)
M openstack-heat/src/main/java/org/jclouds/openstack/heat/v1/HeatApi.java (9)
M openstack-heat/src/main/java/org/jclouds/openstack/heat/v1/domain/Stack.java (17)
A openstack-heat/src/main/java/org/jclouds/openstack/heat/v1/domain/Template.java (57)
M openstack-heat/src/main/java/org/jclouds/openstack/heat/v1/features/StackApi.java (31)
A openstack-heat/src/main/java/org/jclouds/openstack/heat/v1/features/TemplateApi.java (57)
M openstack-heat/src/main/java/org/jclouds/openstack/heat/v1/options/CreateStack.java (113)
M openstack-heat/src/main/java/org/jclouds/openstack/heat/v1/options/UpdateStack.java (66)
M openstack-heat/src/test/java/org/jclouds/openstack/heat/v1/features/StackApiLiveTest.java (54)
M openstack-heat/src/test/java/org/jclouds/openstack/heat/v1/features/StackApiMockTest.java (28)
A openstack-heat/src/test/java/org/jclouds/openstack/heat/v1/features/TemplateApiLiveTest.java (59)
A openstack-heat/src/test/java/org/jclouds/openstack/heat/v1/features/TemplateApiMockTest.java (130)
M openstack-heat/src/test/resources/files_for_atck_template.json (4)
M openstack-heat/src/test/resources/simple_stack.json (4)
A openstack-heat/src/test/resources/stack_post_adopt_request.json (161)
A openstack-heat/src/test/resources/stack_post_adopt_response.json (55)
A openstack-heat/src/test/resources/stack_put_update_request.json (3)
A openstack-heat/src/test/resources/template_get_response.json (50)
A openstack-heat/src/test/resources/template_validate_request.json (3)
A openstack-heat/src/test/resources/template_validate_response.json (34)
M pom.xml (4)
A rackspace-orchestration-uk/README.md (7)
A rackspace-orchestration-uk/pom.xml (131)
A rackspace-orchestration-uk/src/main/java/org/jclouds/rackspace/orchestration/uk/OrchestrationUKProviderMetadata.java (123)
A rackspace-orchestration-uk/src/test/java/org/jclouds/rackspace/orchestration/uk/OrchestrationUKStackApiLiveTest.java (27)
A rackspace-orchestration-uk/src/test/resources/logback.xml (56)
A rackspace-orchestration-us/README.md (7)
A rackspace-orchestration-us/pom.xml (131)
A rackspace-orchestration-us/src/main/java/org/jclouds/rackspace/orchestration/us/OrchestrationUSProviderMetadata.java (128)
A rackspace-orchestration-us/src/test/java/org/jclouds/rackspace/orchestration/us/OrchestrationUSStackApiLiveTest.java (27)
A rackspace-orchestration-us/src/test/resources/logback.xml (56)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-openstack/pull/193.patch
https://github.com/jclouds/jclouds-labs-openstack/pull/193.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -103,7 +103,7 @@
> <dependency>
> <groupId>com.google.auto.value</groupId>
> <artifactId>auto-value</artifactId>
> - <version>1.0</version>
> + <version>1.1</version>
Remove the version to inherit the one configured in the jclouds-project.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193/files#r32462929
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Zack Shoylev <no...@github.com>.
> +import org.jclouds.rest.annotations.PayloadParam;
> +import org.jclouds.rest.annotations.RequestFilters;
> +
> +/**
> + * Provides access to Resource features.
> + *
> + */
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Produces(MediaType.APPLICATION_JSON)
> +public interface TemplateApi {
> +
> + @Named("validation:validate_template")
> + @POST
> + @Path("/validate")
> + @Fallback(NullOnNotFoundOr404.class)
I suspect it might be a valid usage in this case, though, as it is looking for a particular URL to validate. A 404 is also documented as a response code for this sevice call. I will document it in the API.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193/files#r32572875
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Zack Shoylev <no...@github.com>.
I am running a few last tests then will probably rebase, merge, and backport later today.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193#issuecomment-112947158
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Zack Shoylev <no...@github.com>.
> +import org.jclouds.rest.annotations.PayloadParam;
> +import org.jclouds.rest.annotations.RequestFilters;
> +
> +/**
> + * Provides access to Resource features.
> + *
> + */
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Produces(MediaType.APPLICATION_JSON)
> +public interface TemplateApi {
> +
> + @Named("validation:validate_template")
> + @POST
> + @Path("/validate")
> + @Fallback(NullOnNotFoundOr404.class)
I think you are right and I have removed the 404 fallback for the POST call. I can't really justify it, even with the service the way it is right now. The problem is that the service behaves in a somewhat weird way when certain invalid URLs are supplied (socket timeouts in some cases), so this should make more sense overall for now.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193/files#r32673496
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Zack Shoylev <no...@github.com>.
@nacx @andrewgaul If interested, this has the autovalue builders with proper immutable collections. I think this is the pattern we want going forward, but maybe have a look in case we can improve it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193#issuecomment-112125400
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.rest.annotations.PayloadParam;
> +import org.jclouds.rest.annotations.RequestFilters;
> +
> +/**
> + * Provides access to Resource features.
> + *
> + */
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Produces(MediaType.APPLICATION_JSON)
> +public interface TemplateApi {
> +
> + @Named("validation:validate_template")
> + @POST
> + @Path("/validate")
> + @Fallback(NullOnNotFoundOr404.class)
Remove the 404 fallbacks from all POST/PUT methods.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193/files#r32463285
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Ignasi Barrera <no...@github.com>.
>
> public CreateStack build() {
> - CreateStack result = new AutoValue_CreateStack(
> - this.name,
> - this.template,
> - this.templateUrl,
> - this.parameters != null ? ImmutableMap.copyOf(this.parameters) : null,
> - this.disableRollback,
> - this.files != null ? ImmutableMap.copyOf(this.files) : null,
> - this.environment);
> - return result;
> + parameters(getParameters() != null ? ImmutableMap.copyOf(getParameters()) : null);
> + files(getFiles() != null ? ImmutableMap.copyOf(getFiles()) : null);
> + return autoBuild();
This auto builder stuff is super awesome! :D Great stuff!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193/files#r32463536
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Zack Shoylev <no...@github.com>.
> +import org.jclouds.rest.annotations.PayloadParam;
> +import org.jclouds.rest.annotations.RequestFilters;
> +
> +/**
> + * Provides access to Resource features.
> + *
> + */
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Produces(MediaType.APPLICATION_JSON)
> +public interface TemplateApi {
> +
> + @Named("validation:validate_template")
> + @POST
> + @Path("/validate")
> + @Fallback(NullOnNotFoundOr404.class)
I will investigate some more.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193/files#r32573073
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +import org.jclouds.Fallbacks.NullOnNotFoundOr404;
> +import org.jclouds.openstack.heat.v1.domain.Template;
> +import org.jclouds.openstack.keystone.v2_0.filters.AuthenticateRequest;
> +import org.jclouds.rest.annotations.Fallback;
> +import org.jclouds.rest.annotations.Payload;
> +import org.jclouds.rest.annotations.PayloadParam;
> +import org.jclouds.rest.annotations.RequestFilters;
> +
> +/**
> + * Provides access to Resource features.
> + *
> + */
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Produces(MediaType.APPLICATION_JSON)
GET methods don't produce a body, so move it only to the methods that actually need this.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193/files#r32463263
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Ignasi Barrera <no...@github.com>.
Great autobuilder stuff @zack-shoylev! We have to try to update all existing builder boilerplate, it's for sure worth the effort!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193#issuecomment-112204994
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.rest.annotations.PayloadParam;
> +import org.jclouds.rest.annotations.RequestFilters;
> +
> +/**
> + * Provides access to Resource features.
> + *
> + */
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Produces(MediaType.APPLICATION_JSON)
> +public interface TemplateApi {
> +
> + @Named("validation:validate_template")
> + @POST
> + @Path("/validate")
> + @Fallback(NullOnNotFoundOr404.class)
Fallbacks should not be used to "match" what the API specs say. They should be used to configure "what users perceive". In this case, if the 404 is a documented response code, we have to think about how this method will look like to users: it is a method that returns the template when the validation succeeds, but what should it return when the validation fails?
If we use the fallback, a `null` return value may not give enough feedback about where the failure happened: did the validation fail? Did the request fail? I don't know if this could be a valid case in this request. Would it make sense to configure a different fallback that returned a more appropriate exception? If this method is commonly used to just validate, could it return a boolean instead?
Given your input, I don't have a strong opinion on leaving or removing the fallback. Up to you! :) But let's keep this questions in mind to have an API easily understandable and usable by the end users.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193/files#r32579338
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Ignasi Barrera <no...@github.com>.
> +public abstract class Template {
> +
> + /**
> + * @return the description of this Stack Template.
> + */
> + @Nullable public abstract String getDescription();
> +
> + /**
> + * @return the parameters of this Stack, such as compute_flavor or compute_image.
> + */
> + @Nullable public abstract Map<String, Object> getParameters();
> +
> + /**
> + * @return the resources of this Stack, such as server_instance.
> + */
> + @Nullable public abstract Map<String, Object> getResources();
Unless we need this object to have `nullable` collections to "send null elements" (or absent elements) to the target API, we should prefer empty collections to null ones.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193/files#r32463173
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -478,7 +479,8 @@ public void testUpdateStack() throws IOException, InterruptedException {
> */
> assertEquals(server.getRequestCount(), 2);
> assertAuthentication(server);
> - assertRequest(server.takeRequest(), "PUT", BASE_URI + "/stacks/" + TEST_STACK_NAME + "/" + TEST_STACK_ID);
> + assertRequest(server.takeRequest(), "PUT", BASE_URI + "/stacks/" + TEST_STACK_NAME + "/" + TEST_STACK_ID,
> + "/stack_put_update_request.json");
>
> /*
> * Check response
Add also mock and live tests for the new `getTemplate` method.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193/files#r32463687
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Zack Shoylev <no...@github.com>.
> @@ -478,7 +479,8 @@ public void testUpdateStack() throws IOException, InterruptedException {
> */
> assertEquals(server.getRequestCount(), 2);
> assertAuthentication(server);
> - assertRequest(server.takeRequest(), "PUT", BASE_URI + "/stacks/" + TEST_STACK_NAME + "/" + TEST_STACK_ID);
> + assertRequest(server.takeRequest(), "PUT", BASE_URI + "/stacks/" + TEST_STACK_NAME + "/" + TEST_STACK_ID,
> + "/stack_put_update_request.json");
>
> /*
> * Check response
These were included in the PR.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193/files#r32573239
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Zack Shoylev <no...@github.com>.
merged and backported
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193#issuecomment-113276614
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Ignasi Barrera <no...@github.com>.
> + .defaultProperties(OrchestrationUSProviderMetadata.defaultProperties());
> +
> + }
> +
> + @Override
> + public OrchestrationUSProviderMetadata build() {
> + return new OrchestrationUSProviderMetadata(this);
> + }
> +
> + @Override
> + public Builder fromProviderMetadata(ProviderMetadata in) {
> + super.fromProviderMetadata(in);
> + return this;
> + }
> + }
> +}
Add tests for both provider metadata classes (see [the DigitalOcean one](https://github.com/jclouds/jclouds-labs/blob/master/digitalocean/src/test/java/org/jclouds/digitalocean/DigitalOceanProviderMetadataTest.java) for an example).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193/files#r32464273
Re: [jclouds-labs-openstack] Additional support for heat features.
Autovalue refactorings. (#193)
Posted by Zack Shoylev <no...@github.com>.
> +public abstract class Template {
> +
> + /**
> + * @return the description of this Stack Template.
> + */
> + @Nullable public abstract String getDescription();
> +
> + /**
> + * @return the parameters of this Stack, such as compute_flavor or compute_image.
> + */
> + @Nullable public abstract Map<String, Object> getParameters();
> +
> + /**
> + * @return the resources of this Stack, such as server_instance.
> + */
> + @Nullable public abstract Map<String, Object> getResources();
Very good point, but let me expand on that a bit. Classes such as Template here that are returned by deserializing service JSON have no reason to have nullable collections, and it makes sense to just use empty collections. This also should be easy to do.
Classes which we use for serialization to the service should still support nullable collection, as nullables automatically do not get serialized in jclouds. We could change this (i.e. do not serialize empty collections), but I am not sure that makes sense.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/193/files#r32575492