Re: [jclouds-labs-openstack] Poppy service api (#179)
Closed #179. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179#event-250395569
Re: [jclouds-labs-openstack] Poppy service api (#179)
merged --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179#issuecomment-78357174
Re: [jclouds-labs-openstack] Poppy service api (#179)
Will merge when tests pass --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179#issuecomment-78354993
Re: [jclouds-labs-openstack] Poppy service api (#179)
Rebased changes to the original commit --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179#issuecomment-78354886
Re: [jclouds-labs-openstack] Poppy service api (#179)
Last round of sanity checks, then squash and merge. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179#issuecomment-78341462
Re: [jclouds-labs-openstack] Poppy service api (#179)
:+1: --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179#issuecomment-78336509
Re: [jclouds-labs-openstack] Poppy service api (#179)
I have removed the dependency on auto 1.1 and will refactor back to builders once released. At this point I want mostly to make sure this makes it into 1.9.0 --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179#issuecomment-78324705
Re: [jclouds-labs-openstack] Poppy service api (#179)
I'm thinking snapshot is not updated yet. Will try to rebuild later - passes locally. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179#issuecomment-78173346
Re: [jclouds-labs-openstack] Poppy service api (#179)
Applied changes made possible by https://github.com/jclouds/jclouds/pull/700 --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179#issuecomment-78172270
Re: [jclouds-labs-openstack] Poppy service api (#179)
> @@ -52,7 +53,7 @@ > @Consumes(MediaType.APPLICATION_JSON) > @Endpoint(CDN.class) > @RequestFilters(AuthenticateRequest.class) > - @Fallback(FalseOn500or503.class) > + @Fallback(PoppyFallbacks.FalseOn500or503orIOE.class) This PR still needs to be updated after https://github.com/jclouds/jclouds/pull/700 is merged to fix the fallback handler. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r26149980
Re: [jclouds-labs-openstack] Poppy service api (#179)
> +import org.jclouds.json.SerializedNames;
> +
> +import com.google.auto.value.AutoValue;
> +
> +/**
> + * Representation of an OpenStack Poppy Caching Rule.
> + */
> +@AutoValue
> +public abstract class Caching {
> + public abstract String getName();
> + public abstract int getTtl();
> + @Nullable public abstract List getRules();
> +
> + @SerializedNames({ "name", "ttl", "rules" })
> + private static Caching create(String name, int ttl, List
> rules) {
> + return builder().name(name).ttl(ttl).rules(rules).build();
@nacx I am moving the discussion here from IRC.
It's true, using copyOf will ensure that the deserialized object's rules
List getRules() list is immutable.
However, consider this:
Caching caching = Caching.builder().rules(new List()).build();
caching.getRules().add(CachingRule.builder().build());
The generated Builder types have to be the same as the autovalue getter types.
Also, I don't think you can change the behavior of the builder setters.
This is the generated code:
final class AutoValue_Caching extends Caching {
private final String name;
private final int ttl;
private final List rules;
private AutoValue_Caching(
String name,
int ttl,
List rules) {
if (name == null) {
throw new NullPointerException("Null name");
}
this.name = name;
this.ttl = ttl;
this.rules = rules;
}
@Override
public String getName() {
return name;
}
@Override
public int getTtl() {
return ttl;
}
@org.jclouds.javax.annotation.Nullable
@Override
public List getRules() {
return rules;
}
...
@Override
public Caching.Builder toBuilder() {
return new Builder(this);
}
static final class Builder implements Caching.Builder {
private String name;
private Integer ttl;
private List rules;
Builder() {
}
Builder(Caching source) {
name(source.getName());
ttl(source.getTtl());
rules(source.getRules());
}
@Override
public Caching.Builder name(String name) {
this.name = name;
return this;
}
@Override
public Caching.Builder ttl(int ttl) {
this.ttl = ttl;
return this;
}
@Override
public Caching.Builder rules(List rules) {
this.rules = rules;
return this;
}
@Override
public Caching build() {
String missing = "";
if (name == null) {
missing += " name";
}
if (ttl == null) {
missing += " ttl";
}
if (!missing.isEmpty()) {
throw new IllegalStateException("Missing required properties:" +
missing);
}
Caching result = new AutoValue_Caching(
this.name,
this.ttl,
this.rules);
return result;
}
}
}
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r26087263
Re: [jclouds-labs-openstack] Poppy service api (#179)
> + @Named("service:list")
> + @GET
> + @ResponseParser(ParseServices.class)
> + @Transform(ServicesToPagedIterable.class)
> + @Fallback(Fallbacks.EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable list();
> +
> + /**
> +* Lists services by providing a specific set of listing options.
> +* @param options Describes how services should be listed.
> +*/
> + @Named("service:list")
> + @GET
> + @ResponseParser(ParseServices.class)
> + @Fallback(EmptyServicesFallback.class)
> + Services list(PaginationOptions options);
I think the Pagination section might have issues with Fallbacks, but I will
have to verify. Currently it is not in line with it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r26042358
Re: [jclouds-labs-openstack] Poppy service api (#179)
> @@ -52,7 +53,7 @@ > @Consumes(MediaType.APPLICATION_JSON) > @Endpoint(CDN.class) > @RequestFilters(AuthenticateRequest.class) > - @Fallback(FalseOn500or503.class) > + @Fallback(PoppyFallbacks.FalseOn500or503orIOE.class) Thanks @nacx - awesome! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r26042144
Re: [jclouds-labs-openstack] Poppy service api (#179)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + Service get(@PathParam("id") String id);
> +
> + /**
> +* Creates a service.
> +*
> +* @param options the options to create the service with
> +* @return a URI to the created service
> +*/
> + @Named("service:create")
> + @POST
> + @ResponseParser(ParseServiceURIFromHeaders.class)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + URI create(CreateService options);
`options` is an odd name for this param. In `update()` it's called
`updateService`. I suggest `createService `.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r26030007
Re: [jclouds-labs-openstack] Poppy service api (#179)
> +* @param id the id of the service to delete
> +* @return true if delete was successful, false if not
> +*/
> + @Named("network:delete")
> + @DELETE
> + @Path("/{id}")
> + @Fallback(Fallbacks.FalseOnNotFoundOr404.class)
> + boolean delete(@PathParam("id") String id);
> +
> + /**
> +* Updates a service by applying JSONPatch internally.
> +* https://tools.ietf.org/html/rfc6902
> +* This requires providing your updateable JSON and the target JSON.
> +*
> +* @param service Source JSON
> +* @param createService Target JSON
Should be `updateService`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r26029920
Re: [jclouds-labs-openstack] Poppy service api (#179)
> @@ -52,7 +53,7 @@ > @Consumes(MediaType.APPLICATION_JSON) > @Endpoint(CDN.class) > @RequestFilters(AuthenticateRequest.class) > - @Fallback(FalseOn500or503.class) > + @Fallback(PoppyFallbacks.FalseOn500or503orIOE.class) Here is a patch that should fix the issue and allow you to restore the previous fallback that didn't care about IOEs: https://github.com/jclouds/jclouds/pull/700 --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r26029556
Re: [jclouds-labs-openstack] Poppy service api (#179)
> + @Named("service:list")
> + @GET
> + @ResponseParser(ParseServices.class)
> + @Transform(ServicesToPagedIterable.class)
> + @Fallback(Fallbacks.EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable list();
> +
> + /**
> +* Lists services by providing a specific set of listing options.
> +* @param options Describes how services should be listed.
> +*/
> + @Named("service:list")
> + @GET
> + @ResponseParser(ParseServices.class)
> + @Fallback(EmptyServicesFallback.class)
> + Services list(PaginationOptions options);
Should this return a Services object? Is this inline with the [Pagination
section of API/Provider Writing
Practices](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=49578594#API/ProviderWritingPractices-Pagination)?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r26029537
Re: [jclouds-labs-openstack] Poppy service api (#179)
> +
> + @Override
> + public R bindToRequest(R request, Map Object> postParams) {
> + String jsonPatch = null;
> + Service service = (Service) postParams.get("service");
> +
> + Json json = Guice.createInjector(new
> GsonModule()).getInstance(Json.class);
> +
> + String targetService = json.toJson(postParams.get("updateService"));
> + String sourceService =
> json.toJson(service.toUpdatableService().build());
> +
> + ObjectMapper mapper = new ObjectMapper();
> + try {
> + jsonPatch = JsonDiff.asJson(mapper.readTree(sourceService),
> mapper.readTree(targetService)).toString();
> + } catch (IOException e) {
> + e.printStackTrace();
I think this should just be removed completely.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r26029099
Re: [jclouds-labs-openstack] Poppy service api (#179)
> + @Override
> + public R bindToRequest(R request, Map Object> postParams) {
> + String jsonPatch = null;
> + Service service = (Service) postParams.get("service");
> +
> + Json json = Guice.createInjector(new
> GsonModule()).getInstance(Json.class);
> +
> + String targetService = json.toJson(postParams.get("updateService"));
> + String sourceService =
> json.toJson(service.toUpdatableService().build());
> +
> + ObjectMapper mapper = new ObjectMapper();
> + try {
> + jsonPatch = JsonDiff.asJson(mapper.readTree(sourceService),
> mapper.readTree(targetService)).toString();
> + } catch (IOException e) {
> + e.printStackTrace();
> + throw new RuntimeException("Could not create a JSONPatch");
Don't mask the original exception. Use `RuntimeException(String message,
Throwable cause)` instead.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r26029069
Re: [jclouds-labs-openstack] Poppy service api (#179)
> @@ -52,7 +53,7 @@ > @Consumes(MediaType.APPLICATION_JSON) > @Endpoint(CDN.class) > @RequestFilters(AuthenticateRequest.class) > - @Fallback(FalseOn500or503.class) > + @Fallback(PoppyFallbacks.FalseOn500or503orIOE.class) I've investigated a bit, and the issue you are facing is [JCLOUDS-532](https://issues.apache.org/jira/browse/JCLOUDS-532). With the Java default driver does not fail, but it seems to fail with the Apache HC driver (the one used in the reported issue) and OkHttp. Both seem to use custom `InputStreams` that throw an `IOException` when you call `read`, while he default implementation seems to just return -1. That read happens later in the error handler when trying to build the exception to propagate. I'd like to investigate this a bit more, as there is a [explicit test to verify that the InputStream is closed](https://github.com/jclouds/jclouds/blob/master/core/src/test/java/org/jclouds/http/handlers/BackoffLimitedRetryHandlerTest.java#L119-L160), so we need more background to determine if we can relax that and don't close the stream there. I've assigned the issue to me and I expect to have a look at it this week, but in any case, I think we should revert the fallback to the previous one. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r26009050
Re: [jclouds-labs-openstack] Poppy service api (#179)
Thanks for the review @nacx! Good stuff. Any idea on who to contact about the google auto 1.1 release? I don't want to bother them too much, especially if we already asked. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179#issuecomment-77651367
Re: [jclouds-labs-openstack] Poppy service api (#179)
> +import org.jclouds.json.SerializedNames;
> +
> +import com.google.auto.value.AutoValue;
> +
> +/**
> + * Representation of an OpenStack Poppy Caching Rule.
> + */
> +@AutoValue
> +public abstract class Caching {
> + public abstract String getName();
> + public abstract int getTtl();
> + @Nullable public abstract List getRules();
> +
> + @SerializedNames({ "name", "ttl", "rules" })
> + private static Caching create(String name, int ttl, List
> rules) {
> + return builder().name(name).ttl(ttl).rules(rules).build();
The reason I left it here as is: this only gets called by GSON when
deserializing (afaik). Thoughts on the trade-off? Probably just safer to change
it to copy.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25985025
Re: [jclouds-labs-openstack] Poppy service api (#179)
> @@ -52,7 +53,7 @@ > @Consumes(MediaType.APPLICATION_JSON) > @Endpoint(CDN.class) > @RequestFilters(AuthenticateRequest.class) > - @Fallback(FalseOn500or503.class) > + @Fallback(PoppyFallbacks.FalseOn500or503orIOE.class) testPingFailOn500(org.jclouds.openstack.poppy.v1.features.PoppyApiMockTest) Time elapsed: 0.173 sec <<< FAILURE! org.jclouds.http.HttpResponseException: java.io.IOException: closed connecting to GET http://localhost:56553/v1.0/123123/ping HTTP/1.1 at okio.RealBufferedSource$1.read(RealBufferedSource.java:294) at java.io.InputStream.read(InputStream.java:101) at com.google.common.io.ByteStreams.copy(ByteStreams.java:175) at com.google.common.io.ByteStreams.toByteArray(ByteStreams.java:220) at org.jclouds.http.HttpUtils.toByteArrayOrNull(HttpUtils.java:125) at org.jclouds.http.HttpUtils.closeClientButKeepContentStream(HttpUtils.java:159) at org.jclouds.openstack.poppy.v1.handlers.PoppyErrorHandler.handleError(PoppyErrorHandler.java:32) at org.jclouds.http.handlers.DelegatingErrorHandler.handleError(DelegatingErrorHandler.java:67) at org.jclouds.http.internal.BaseHttpCommandExecutorService.shouldContinue(BaseHttpCommandExecutorService.java:128) at org.jclouds.http.internal.BaseHttpCommandExecutorService.invoke(BaseHttpCommandExecutorService.java:98) at org.jclouds.rest.internal.InvokeHttpMethod.invoke(InvokeHttpMethod.java:90) at org.jclouds.rest.internal.InvokeHttpMethod.apply(InvokeHttpMethod.java:73) at org.jclouds.rest.internal.InvokeHttpMethod.apply(InvokeHttpMethod.java:44) at org.jclouds.rest.internal.DelegatesToInvocationFunction.handle(DelegatesToInvocationFunction.java:156) at org.jclouds.rest.internal.DelegatesToInvocationFunction.invoke(DelegatesToInvocationFunction.java:123) at com.sun.proxy.$Proxy52.ping(Unknown Source) at org.jclouds.openstack.poppy.v1.features.PoppyApiMockTest.testPingFailOn500(PoppyApiMockTest.java:62) This is tested by returning 500 from mockhttp and then trying to process it with returnValueOnCodeOrNull --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25984591
Re: [jclouds-labs-openstack] Poppy service api (#179)
> + * Diff to create JSONPatch using dependency.
> + * Send the JSONPatch in the request.
> + *
> + * JSONPatch RFC:
> + * https://tools.ietf.org/html/rfc6902
> + */
> +public class JSONPatchUpdate implements MapBinder {
> + @Inject
> + private BindToJsonPayload jsonBinder;
> +
> + @Override
> + public R bindToRequest(R request, Map Object> postParams) {
> + String jsonPatch = null;
> + Service service = (Service) postParams.get("service");
> +
> + Json json = Guice.createInjector(new
> GsonModule()).getInstance(Json.class);
Inject the Json object normally instead of creating a new entire injector.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25982313
Re: [jclouds-labs-openstack] Poppy service api (#179)
> @@ -46,6 +46,20 @@
>
> org.jclouds*;version="${project.version}",*
>
>
> +
Absolutely. That would help us remove a lot of boilerplate code. Let's try to
push them to make a release!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25982906
Re: [jclouds-labs-openstack] Poppy service api (#179)
> + @GET
> + @Path("/{id}")
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + Service get(@PathParam("id") String id);
> +
> + /**
> +* Creates a service.
> +*
> +* @param options the options to create the service with
> +* @return a URI to the created service
> +*/
> + @Named("service:create")
> + @POST
> + @ResponseParser(ParseServiceURIFromHeaders.class)
> + @Fallback(NullOnNotFoundOr404.class)
We shouldn't use 440 fallbacks in POST/PUT operations. There is already
[JCLOUDS-691](https://issues.apache.org/jira/browse/JCLOUDS-691) an issue to
remove them all, but we should not proliferate them.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25982162
Re: [jclouds-labs-openstack] Poppy service api (#179)
> +
> + @Override
> + public R bindToRequest(R request, Map Object> postParams) {
> + String jsonPatch = null;
> + Service service = (Service) postParams.get("service");
> +
> + Json json = Guice.createInjector(new
> GsonModule()).getInstance(Json.class);
> +
> + String targetService = json.toJson(postParams.get("updateService"));
> + String sourceService =
> json.toJson(service.toUpdatableService().build());
> +
> + ObjectMapper mapper = new ObjectMapper();
> + try {
> + jsonPatch = JsonDiff.asJson(mapper.readTree(sourceService),
> mapper.readTree(targetService)).toString();
> + } catch (IOException e) {
> + e.printStackTrace();
Use a logger instead of printing directly to the console.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25982345
Re: [jclouds-labs-openstack] Poppy service api (#179)
> +import javax.inject.Singleton;
> +
> +import org.jclouds.http.functions.ParseJson;
> +import org.jclouds.json.Json;
> +import org.jclouds.openstack.poppy.v1.domain.Services;
> +
> +import com.google.inject.TypeLiteral;
> +
> +/**
> + * Used by jclouds to provide more specific collections and fallbacks.
> + */
> +@Singleton
> +public class ParseServices extends ParseJson {
> +
> + @Inject
> + public ParseServices(Json json) {
In general, all injection constructors should be package private.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25982241
Re: [jclouds-labs-openstack] Poppy service api (#179)
> @@ -52,7 +53,7 @@ > @Consumes(MediaType.APPLICATION_JSON) > @Endpoint(CDN.class) > @RequestFilters(AuthenticateRequest.class) > - @Fallback(FalseOn500or503.class) > + @Fallback(PoppyFallbacks.FalseOn500or503orIOE.class) Let me try to reproduce it again then... --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25982086
Re: [jclouds-labs-openstack] Poppy service api (#179)
> +
> + /**
> +* Updates a service by applying JSONPatch internally.
> +* https://tools.ietf.org/html/rfc6902
> +* This requires providing your updateable JSON and the target JSON.
> +*
> +* @param service Source JSON
> +* @param createService Target JSON
> +* @return a URI to the created service
> +*/
> + @Named("service:update")
> + @PATCH
> + @Path("/{id}")
> + @ResponseParser(ParseServiceURIFromHeaders.class)
> + @MapBinder(JSONPatchUpdate.class)
> + @Fallback(NullOnNotFoundOr404.class)
Same comment about 404 fallbacks in "wrote" operations.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25982198
Re: [jclouds-labs-openstack] Poppy service api (#179)
> @@ -46,6 +46,20 @@
>
> org.jclouds*;version="${project.version}",*
>
>
> +
Almost a recurring theme at this point... :) But it's just so powerful!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25982047
Re: [jclouds-labs-openstack] Poppy service api (#179)
> +import java.util.List;
> +import java.util.Set;
> +
> +import org.jclouds.javax.annotation.Nullable;
> +import org.jclouds.json.SerializedNames;
> +import org.jclouds.openstack.v2_0.domain.Link;
> +
> +import com.google.auto.value.AutoValue;
> +import com.google.inject.name.Named;
> +
> +/**
> + * Representation of an OpenStack Poppy Service.
> + */
> +@AutoValue
> +public abstract class Service {
> + @Named("id") @Nullable public abstract String getId();
Is the `@Named` annotation needed here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25981957
Re: [jclouds-labs-openstack] Poppy service api (#179)
> +import java.util.List;
> +
> +import org.jclouds.javax.annotation.Nullable;
> +import org.jclouds.json.SerializedNames;
> +
> +import com.google.auto.value.AutoValue;
> +import com.google.common.collect.ImmutableList;
> +
> +/**
> + * Representation of an OpenStack Poppy Origin.
> + */
> +@AutoValue
> +public abstract class Origin {
> + public abstract String getOrigin();
> + @Nullable public abstract int getPort();
> + @Nullable public abstract boolean getSslEnabled();
Primitive types can't be null.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25981795
Re: [jclouds-labs-openstack] Poppy service api (#179)
> +import org.jclouds.json.SerializedNames;
> +
> +import com.google.auto.value.AutoValue;
> +
> +/**
> + * Representation of an OpenStack Poppy Caching Rule.
> + */
> +@AutoValue
> +public abstract class Caching {
> + public abstract String getName();
> + public abstract int getTtl();
> + @Nullable public abstract List getRules();
> +
> + @SerializedNames({ "name", "ttl", "rules" })
> + private static Caching create(String name, int ttl, List
> rules) {
> + return builder().name(name).ttl(ttl).rules(rules).build();
To enforce immutability also in lists use `ImmutableList.copyOf` when the list
is not null.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25981666
Re: [jclouds-labs-openstack] Poppy service api (#179)
> @@ -46,6 +46,20 @@
>
> org.jclouds*;version="${project.version}",*
>
>
> +
Fine, but as discussed in other threads take care and don't merge this pull
request until that happens :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25981590
Re: [jclouds-labs-openstack] Poppy service api (#179)
> @@ -52,7 +53,7 @@ > @Consumes(MediaType.APPLICATION_JSON) > @Endpoint(CDN.class) > @RequestFilters(AuthenticateRequest.class) > - @Fallback(FalseOn500or503.class) > + @Fallback(PoppyFallbacks.FalseOn500or503orIOE.class) Can you share the stacktrace? This seems to be something that should be fixed in the OkHttp driver itself. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25981537
Re: [jclouds-labs-openstack] Poppy service api (#179)
> + * 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.openstack.poppy.v1.domain;
> +
> +import org.jclouds.json.SerializedNames;
> +
> +import com.google.auto.value.AutoValue;
> +
> +@AutoValue
> +public abstract class CachingRule {
> + public abstract String getName();
This PR needs more javadoc.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25978545
Re: [jclouds-labs-openstack] Poppy service api (#179)
> +import org.jclouds.json.config.GsonModule; > +import org.jclouds.openstack.poppy.v1.domain.Service; > +import org.jclouds.rest.MapBinder; > +import org.jclouds.rest.binders.BindToJsonPayload; > + > +import com.fasterxml.jackson.databind.ObjectMapper; > +import com.github.fge.jsonpatch.diff.JsonDiff; > +import com.google.inject.Guice; > +import com.google.inject.Inject; > + > +/** > + * This will create a JSONPatch out of a Service and an UpdateService. > + * > + * User side: > + * Get a Service with api.get(service_id) > + * Get a UpdateService builder by using Service.toUpdatabaleService() typo --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25956551
Re: [jclouds-labs-openstack] Poppy service api (#179)
>provided
>
> +
> + org.apache.jclouds.driver
> + jclouds-okhttp
> + ${project.parent.version}
> +
> +
> + com.github.fge
> + json-patch
> + 1.9
> +
This api's update feature uses PATCH that accepts a JSONPatch as per the RFC
https://tools.ietf.org/html/rfc6902
This is where most of the complexity arises. We want users to deal with our
domain objects, but still send the service a proper JSONPatch diff.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25899569
Re: [jclouds-labs-openstack] Poppy service api (#179)
> @@ -52,7 +53,7 @@ > @Consumes(MediaType.APPLICATION_JSON) > @Endpoint(CDN.class) > @RequestFilters(AuthenticateRequest.class) > - @Fallback(FalseOn500or503.class) > + @Fallback(PoppyFallbacks.FalseOn500or503orIOE.class) Switching to okhttp: 500 errors seem to cause an IOException instead - slightly different behavior. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25899320
Re: [jclouds-labs-openstack] Poppy service api (#179)
> @@ -46,6 +46,20 @@
>
> org.jclouds*;version="${project.version}",*
>
>
> +
This PR uses autovalue 1.1 snapshot for now (until released)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25899272
Re: [jclouds-labs-openstack] Poppy service api (#179)
> + * User side: > + * Get a Service with api.get(service_id) > + * Get a UpdateService builder by using Service.toUpdatabaleService() > + *This step will provide an interface that exposes the updatable JSON > values to the user. > + * Use the UpdateService.Builder instance to modify and build() a new > UpdateService. > + * Send the original Service and the new UpdateService to the api.update > method. > + * > + * jclouds side: > + * Convert the Service to UpdateService, but don't change it (this is the > source). > + * Serialize both source and target to String > + * Diff to create JSONPatch using dependency. > + * Send the JSONPatch in the request. > + * > + * JSONPatch RFC: > + * https://tools.ietf.org/html/rfc6902 > + */ The explanation above should be mostly sufficient, but I was wondering if there is a better way to handle JSONPatch --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/179/files#r25899397
