You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Adrian Cole <no...@github.com> on 2014/10/26 20:29:17 UTC
[jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker. (#102)
this builds on https://github.com/jclouds/jclouds/pull/586 and depends on it.
You can merge this Pull Request by running:
git pull https://github.com/adriancole/jclouds-labs adrian.pilot-auto-docker
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs/pull/102
-- Commit Summary --
* JCLOUDS-750 Start using AutoValue on Docker.
-- File Changes --
M docker/pom.xml (7)
M docker/src/main/java/org/jclouds/docker/compute/strategy/DockerComputeServiceAdapter.java (4)
M docker/src/main/java/org/jclouds/docker/config/DockerParserModule.java (38)
M docker/src/main/java/org/jclouds/docker/domain/Config.java (30)
M docker/src/main/java/org/jclouds/docker/domain/Container.java (36)
M docker/src/main/java/org/jclouds/docker/domain/ExposedPorts.java (9)
M docker/src/main/java/org/jclouds/docker/domain/HostConfig.java (17)
M docker/src/main/java/org/jclouds/docker/domain/Image.java (227)
M docker/src/main/java/org/jclouds/docker/domain/NetworkSettings.java (21)
M docker/src/main/java/org/jclouds/docker/domain/Port.java (14)
M docker/src/main/java/org/jclouds/docker/domain/State.java (12)
M docker/src/main/java/org/jclouds/docker/domain/Version.java (12)
M docker/src/test/java/org/jclouds/docker/compute/functions/ImageToImageTest.java (20)
M docker/src/test/java/org/jclouds/docker/config/DockerParserModuleTest.java (25)
M docker/src/test/java/org/jclouds/docker/features/RemoteApiMockTest.java (1)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/102.patch
https://github.com/jclouds/jclouds-labs/pull/102.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
> bind(GsonModule.DateAdapter.class).to(GsonModule.Iso8601DateAdapter.class);
> }
> -
> - @Provides
> - @Singleton
> - public Map<Type, Object> provideCustomAdapterBindings() {
> - return new ImmutableMap.Builder<Type, Object>()
> - .put(Container.class, new ContainerTypeAdapter())
> - .build();
> - }
> -
> - protected static class ContainerTypeAdapter implements JsonDeserializer<Container> {
> -
> - @Override
> - public Container deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws
> - JsonParseException {
> - Gson gson = new GsonBuilder().serializeNulls().create();
https://github.com/adriancole/jclouds/commit/49d7cd444b59f41ee8901208b3ad5156767b80f5
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102/files#r19383089
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
> bind(GsonModule.DateAdapter.class).to(GsonModule.Iso8601DateAdapter.class);
> }
> -
> - @Provides
> - @Singleton
> - public Map<Type, Object> provideCustomAdapterBindings() {
> - return new ImmutableMap.Builder<Type, Object>()
> - .put(Container.class, new ContainerTypeAdapter())
> - .build();
> - }
> -
> - protected static class ContainerTypeAdapter implements JsonDeserializer<Container> {
> -
> - @Override
> - public Container deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws
> - JsonParseException {
> - Gson gson = new GsonBuilder().serializeNulls().create();
this was hiding a bug in core!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102/files#r19383064
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #357](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/357/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102#issuecomment-60713237
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
Closed #102.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102#event-184721827
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
Reopened #102.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102#event-184721837
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Andrea Turli <no...@github.com>.
> private final List<String> onBuild;
>
> -
> - @ConstructorProperties({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
> + @SerializedNames({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
Docker APIs contain some inconsistencies like that so maybe we could need 2 objects i.e. ContainerRequest and ContainerResponse maybe?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102/files#r19445850
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #1811](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/1811/) 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/pull/102#issuecomment-60719862
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
> private final List<String> onBuild;
>
> -
> - @ConstructorProperties({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
> + @SerializedNames({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
@gk5885 @eamonnmcmanus @cgruber FYI: I updated our custom gson integration so that it can recognize factory methods and pluck the names off. Would love to replace this with generated code at some point, but it is currently reflective.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102/files#r19436551
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #351](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/351/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102#issuecomment-60547308
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
> private final List<String> onBuild;
>
> -
> - @ConstructorProperties({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
> + @SerializedNames({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
yeah the field names should correspond directly to the naming convention of
the api. If there are differences, we should fix them. Mind pinging them?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102/files#r19446062
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
@andreaturli ptal
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102#issuecomment-60599465
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #1799](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/1799/) 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/pull/102#issuecomment-60547855
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #1814](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/1814/) 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/pull/102#issuecomment-60770480
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #350](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/350/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102#issuecomment-60528926
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
cc @jdaggett @andreaturli
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102#issuecomment-60528856
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
thx for the review, @nacx!
kicking to make sure it works on latest snapshot.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102#issuecomment-60767826
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
> private final List<String> onBuild;
>
> -
> - @ConstructorProperties({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
> + @SerializedNames({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
@nacx fyi I'll update this PR with the case format from the latest api. Also, I will look at using the SerializedNames annotation for serialization, too. It isn't as easy to get as it was for deserialization, but not impossible.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102/files#r19446379
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
> - @Singleton
> - public Map<Type, Object> provideCustomAdapterBindings() {
> - return new ImmutableMap.Builder<Type, Object>()
> - .put(Container.class, new ContainerTypeAdapter())
> - .build();
> - }
> -
> - protected static class ContainerTypeAdapter implements JsonDeserializer<Container> {
> -
> - @Override
> - public Container deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws
> - JsonParseException {
> - Gson gson = new GsonBuilder().serializeNulls().create();
> - final JsonObject jsonObject = json.getAsJsonObject();
> - return gson.fromJson(jsonObject, Container.class);
> + /** When serializing, Most fields are UpperCamelCase, with some exceptions. */
@nacx @jdaggett @zack-shoylev this is how you screw with naming policy.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102/files#r19455895
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Ignasi Barrera <no...@github.com>.
> private final List<String> onBuild;
>
> -
> - @ConstructorProperties({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
> + @SerializedNames({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
It seems the naming was OK: https://docs.docker.com/reference/api/docker_remote_api_v1.12/#create-a-container
I've quickly checked the latest version of the API and the naming hasn't changed.
Worth renaming the fields, even if they logo ugly, to keep the serialization based on the naming configuration?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102/files#r19445928
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
> @Override
> protected void configure() {
> + bind(FieldNamingPolicy.class).toInstance(FieldNamingPolicy.UPPER_CAMEL_CASE);
@nacx in the future, when serializednames is used for serialization, too, the above will be redundant.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102/files#r19446646
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #361](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/361/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102#issuecomment-60768671
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Ignasi Barrera <no...@github.com>.
> private final List<String> onBuild;
>
> -
> - @ConstructorProperties({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
> + @SerializedNames({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
Out of curiosity. Is this object intended to be serialized? If so, when serializing, is the serialized form for each field extracted somehow from the constructor annotation?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102/files#r19441121
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
This depends on latest jclouds snapshot. now removes 1K lines as I finished using auto on all objects.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102#issuecomment-60718195
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
> private final List<String> onBuild;
>
> -
> - @ConstructorProperties({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
> + @SerializedNames({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
Can you prove that by capturing test data, andrea? As we drift towards
latest version of docker api, I don't want to maintain 2 objects or a
custom type adapter because of something on an earlier version.
Ex. can you prove that the latest version of docker returns ImageId yet
requires you to serialize the field as imageId (or similar)?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102/files#r19446133
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #359](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/359/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102#issuecomment-60718353
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
cherry-picked into master, (caught 1.8.x back up, then) cherry-picked into 1.8.x
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102#issuecomment-60773727
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #1793](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/1793/) FAILURE
Looks like there's a problem with this pull request
[(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/pull/102#issuecomment-60529264
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #1808](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/1808/) 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/pull/102#issuecomment-60713765
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Adrian Cole <no...@github.com>.
> private final List<String> onBuild;
>
> -
> - @ConstructorProperties({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
> + @SerializedNames({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
Serialization is covered by the naming policy bound in the parser module.
In this case, upper camel!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102/files#r19444561
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker. (#102)
Posted by Adrian Cole <ad...@gmail.com>.
That looks like a latent typo on the serialized name field. No test exists
to confirm or refute except we could look at docker docs or code.
Would be weird that the caseformat is different on input vs output, right?
If in the odd case input is imageId and output is ImageId, then yeah...
this wouldn't work.
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Ignasi Barrera <no...@github.com>.
> private final List<String> onBuild;
>
> -
> - @ConstructorProperties({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
> + @SerializedNames({ "Hostname", "Domainname", "User", "Memory", "MemorySwap", "CpuShares", "AttachStdin",
Hmmm Will it work for the `imageId` and `domainName` fields too, given their (removed) SerializedName?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102/files#r19445264
Re: [jclouds-labs] JCLOUDS-750 Start using AutoValue on Docker.
(#102)
Posted by Ignasi Barrera <no...@github.com>.
Changes LGTM!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/102#issuecomment-60736481