You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Jeremy Daggett <no...@github.com> on 2015/02/06 21:12:22 UTC
[jclouds-labs-openstack] Initial support for OpenStack Poppy Service
API (#178)
This PR is intended to get feedback from the community on the new builder support. It does _not_ include live tests.
- Introduces `AutoValue.Builder` support to jclouds
- Adds `Service` and related domain objects
- Adds Poppy `ServiceApi` create and get methods
- Adds `ServiceApiMockTest`
/cc @zack-shoylev @nacx @demobox
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs-openstack/pull/178
-- Commit Summary --
* JCLOUDS-806: Support OpenStack Poppy Service API and introduce AutoValue.Builder support
-- File Changes --
M openstack-poppy/pom.xml (1)
M openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/PoppyApi.java (6)
A openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/domain/Caching.java (82)
A openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/domain/Domain.java (70)
A openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/domain/Error.java (40)
A openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/domain/Origin.java (85)
M openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/domain/Provider.java (6)
A openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/domain/Restriction.java (82)
A openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/domain/Service.java (85)
M openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/features/FlavorApi.java (16)
A openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/features/ServiceApi.java (78)
A openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/functions/ParseServiceURIFromHeaders.java (43)
A openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/options/ServiceCreateOptions.java (67)
A openstack-poppy/src/test/java/org/jclouds/openstack/poppy/v1/features/ServiceApiMockTest.java (130)
A openstack-poppy/src/test/resources/poppy_service_create_request.json (37)
A openstack-poppy/src/test/resources/poppy_service_get_response.json (72)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-openstack/pull/178.patch
https://github.com/jclouds/jclouds-labs-openstack/pull/178.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/178
Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy
Service API (#178)
Posted by Ignasi Barrera <no...@github.com>.
Agree @zack-shoylev. Let's add the `toBuilder` just where it makes sense.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/178#issuecomment-73471782
Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy
Service API (#178)
Posted by Zack Shoylev <no...@github.com>.
> @@ -90,6 +90,7 @@
> <dependency>
> <groupId>com.google.auto.value</groupId>
> <artifactId>auto-value</artifactId>
> + <version>1.0-SNAPSHOT</version>
But autovalue is not included as a dependency anyways - just the generated code. As long as the generated code passes the tests, I see no problem. Thoughts?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/178/files#r24353079
Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy
Service API (#178)
Posted by Zack Shoylev <no...@github.com>.
I think I have addressed most of the issues around this PR and created new ones in https://github.com/jclouds/jclouds-labs-openstack/pull/179
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/178#issuecomment-77446637
Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy
Service API (#178)
Posted by Ignasi Barrera <no...@github.com>.
I've just looked at the model classes and this is really cool and very clean! Just one minor, according to the auto docs, you could add a non-static method that returns the Builder (typically toBuilder()) and auto will mplement it and populate the builder with the current values. It's just one line worth adding for read/write objects. WDYT?
The builder thing looks really great (I'll look at the rest of the PR with more detail later) so let's use it in the other repos if it works well. Thanks @jdaggett!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/178#issuecomment-73332649
Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy
Service API (#178)
Posted by Andrew Gaul <no...@github.com>.
> @@ -90,6 +90,7 @@
> <dependency>
> <groupId>com.google.auto.value</groupId>
> <artifactId>auto-value</artifactId>
> + <version>1.0-SNAPSHOT</version>
auto-value released 1.0 recently:
http://repo1.maven.org/maven2/com/google/auto/value/auto-value/
Or do we need features only included in master/1.1-SNAPSHOT?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/178/files#r24366000
Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy
Service API (#178)
Posted by Zack Shoylev <no...@github.com>.
I have a small counter-argument about the toBuilder that I have been struggling with:
You have 2 types of domain objects: write and read. For example:
Flavor flavor = api.list().first(); // <-- Only generated by parsing service gson
vs
api.create(CreateFlavor.builder().name("2").build()); // <-- Always generated by the user using the builder
Specifically, you can't meaningfully do
Flavor flavor = api.list().first();
api.create(flavor.toBuilder().name("some new name").build()); // as the service will return additional properties and the two domain objects are conceptually different.
Which is why I think this might be confusing. If you managed to follow that reasoning, I'd like to hear some thoughts on it :)
We should probably just provide it as it's an easy convenience method that might be useful in some cases?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/178#issuecomment-73346304
Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy
Service API (#178)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -90,6 +90,7 @@
> <dependency>
> <groupId>com.google.auto.value</groupId>
> <artifactId>auto-value</artifactId>
> + <version>1.0-SNAPSHOT</version>
Take into account that we release **the source code**. Binaries (and Maven artifacts) are provided just for convenience. The official release is only the source code, and that can't depend on SNAPSHOT versions.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/178/files#r24363738
Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy
Service API (#178)
Posted by Zack Shoylev <no...@github.com>.
> @@ -90,6 +90,7 @@
> <dependency>
> <groupId>com.google.auto.value</groupId>
> <artifactId>auto-value</artifactId>
> + <version>1.0-SNAPSHOT</version>
The builders are in 1.1 which is what we need to make this beautiful. As autovalue builders are also the way to go forward, this seems to be the minimum we need.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/178/files#r24452468
Re: [jclouds-labs-openstack] Initial support for OpenStack Poppy
Service API (#178)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -90,6 +90,7 @@
> <dependency>
> <groupId>com.google.auto.value</groupId>
> <artifactId>auto-value</artifactId>
> + <version>1.0-SNAPSHOT</version>
We have to be *very* careful about this, and I wouldn't merge this PR until there is a release of auto with the builder support. Take into account that we **can't** release jclouds with SNAPSHOT dependencies.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/178/files#r24313703