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 2014/06/18 01:43:57 UTC
[jclouds-labs-openstack] Neutron Refactoring (#111)
v2_0 -> v2
No options (redesign)
More features (extension-related)
Some bugfixes
You can merge this Pull Request by running:
git pull https://github.com/rackerlabs/jclouds-labs-openstack neutron-refactoring
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-openstack/pull/111
-- Commit Summary --
* Neutron Refactoring
-- File Changes --
M openstack-neutron/pom.xml (5)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/NeutronApi.java (81)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/NeutronApiMetadata.java (101)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/config/NeutronHttpApiModule.java (84)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/AddressPair.java (130)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/AllocationPool.java (132)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/CreateNetwork.java (491)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/CreatePort.java (544)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/CreateRouter.java (183)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/CreateSubnet.java (345)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/ExternalGatewayInfo.java (110)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/ExtraDhcpOption.java (160)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/HostRoute.java (117)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/IP.java (131)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/IPv6DHCPMode.java (54)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/Network.java (96)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/NetworkSegment.java (162)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/NetworkStatus.java (54)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/NetworkType.java (57)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/Networks.java (35)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/Port.java (135)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/Ports.java (35)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/Router.java (79)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/RouterInterface.java (135)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/Routers.java (35)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/Subnet.java (68)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/Subnets.java (35)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/VIFType.java (59)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/VNICType.java (53)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/extensions/RouterApi.java (196)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/fallbacks/EmptyNetworksFallback.java (45)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/fallbacks/EmptyPortsFallback.java (45)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/fallbacks/EmptyRoutersFallback.java (45)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/fallbacks/EmptySubnetsFallback.java (45)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/features/NetworkApi.java (152)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/features/PortApi.java (153)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/features/SubnetApi.java (149)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/functions/NetworksToPagedIterable.java (64)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/functions/ParseNetworks.java (38)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/functions/ParsePorts.java (37)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/functions/ParseRouters.java (37)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/functions/ParseSubnets.java (37)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/functions/PortsToPagedIterable.java (64)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/functions/RouterToPagedIterable.java (66)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/functions/SubnetsToPagedIterable.java (64)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/handlers/NeutronErrorHandler.java (59)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/options/EmptyOptions.java (47)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/NeutronApi.java (2)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/NeutronApiMetadata.java (3)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/config/NeutronHttpApiModule.java (30)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/extensions/RouterApi.java (2)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/features/NetworkApi.java (2)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/features/PortApi.java (2)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/features/SubnetApi.java (2)
M openstack-neutron/src/main/resources/META-INF/services/org.jclouds.apis.ApiMetadata (1)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2/NeutronApiMetadataTest.java (35)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2/features/NetworkApiMockTest.java (490)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2/features/PortApiMockTest.java (506)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2/features/RouterApiMockTest.java (642)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2/features/SubnetApiMockTest.java (498)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2/internal/BaseNeutronApiExpectTest.java (29)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2/internal/BaseNeutronApiLiveTest.java (46)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2/internal/BaseNeutronApiMockTest.java (41)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2/internal/BaseNeutronExpectTest.java (66)
M openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/internal/BaseNeutronApiLiveTest.java (2)
M openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/internal/BaseNeutronExpectTest.java (2)
A openstack-neutron/src/test/resources/access.json (228)
A openstack-neutron/src/test/resources/network_bulk_create_request.json (12)
A openstack-neutron/src/test/resources/network_bulk_create_response.json (18)
A openstack-neutron/src/test/resources/network_create_request.json (4)
A openstack-neutron/src/test/resources/network_create_response.json (7)
A openstack-neutron/src/test/resources/network_get_response.json (7)
A openstack-neutron/src/test/resources/network_list_response.json (62)
A openstack-neutron/src/test/resources/network_list_response_paged1.json (40)
A openstack-neutron/src/test/resources/network_list_response_paged2.json (36)
A openstack-neutron/src/test/resources/network_update_request.json (4)
A openstack-neutron/src/test/resources/network_update_response.json (12)
A openstack-neutron/src/test/resources/port_create_bulk_request.json (22)
A openstack-neutron/src/test/resources/port_create_bulk_response.json (47)
A openstack-neutron/src/test/resources/port_create_request.json (8)
A openstack-neutron/src/test/resources/port_create_response.json (19)
A openstack-neutron/src/test/resources/port_get_response.json (7)
A openstack-neutron/src/test/resources/port_list_response.json (86)
A openstack-neutron/src/test/resources/port_list_response_paged1.json (48)
A openstack-neutron/src/test/resources/port_list_response_paged2.json (44)
A openstack-neutron/src/test/resources/port_update_request.json (8)
A openstack-neutron/src/test/resources/port_update_response.json (23)
M openstack-neutron/src/test/resources/router.json (2)
A openstack-neutron/src/test/resources/router_add_interface_port_request.json (3)
A openstack-neutron/src/test/resources/router_add_interface_request.json (3)
A openstack-neutron/src/test/resources/router_add_interface_response.json (4)
A openstack-neutron/src/test/resources/router_create_request.json (9)
A openstack-neutron/src/test/resources/router_create_response.json (12)
A openstack-neutron/src/test/resources/router_get_response.json (14)
A openstack-neutron/src/test/resources/router_list_response.json (74)
A openstack-neutron/src/test/resources/router_list_response_paged1.json (34)
A openstack-neutron/src/test/resources/router_list_response_paged2.json (30)
A openstack-neutron/src/test/resources/router_remove_interface_port_request.json (3)
A openstack-neutron/src/test/resources/router_remove_interface_subnet_request.json (3)
A openstack-neutron/src/test/resources/router_update_request.json (7)
A openstack-neutron/src/test/resources/router_update_response.json (12)
A openstack-neutron/src/test/resources/subnet_bulk_create_request.json (14)
A openstack-neutron/src/test/resources/subnet_bulk_create_response.json (48)
A openstack-neutron/src/test/resources/subnet_create_request.json (6)
A openstack-neutron/src/test/resources/subnet_create_response.json (8)
A openstack-neutron/src/test/resources/subnet_get_response.json (8)
A openstack-neutron/src/test/resources/subnet_list_response.json (98)
A openstack-neutron/src/test/resources/subnet_list_response_pages1.json (30)
A openstack-neutron/src/test/resources/subnet_list_response_pages2.json (26)
A openstack-neutron/src/test/resources/subnet_update_request.json (7)
A openstack-neutron/src/test/resources/subnet_update_response.json (14)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-openstack/pull/111.patch
https://github.com/jclouds/jclouds-labs-openstack/pull/111.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + @SelectJson("router")
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + @Nullable
> + Router update(@PathParam("id") String id, @WrapWith("router") Router.UpdateOptions router);
> +
> + /**
> + * Deletes the specified router
> + *
> + * @param id the id of the router to delete
> + * @return true if delete successful, false if not
> + */
> + @Named("router:delete")
> + @DELETE
> + @Path("/{id}")
> + @Fallback(Fallbacks.FalseOnNotFoundOr404.class)
> + boolean delete(@PathParam("id") String id);
We should use primitives where possible, like here.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15361497
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers list(PaginationOptions options);
> +
> + /**
> + * Returns a Routers collection that should contain a single router with the id requested.
> + *
> + * @param id the id of the router to return
> + * @return Routers collection or empty if not found
> + */
> + @Named("router:get")
> + @GET
> + @Path("/{id}")
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers get(@PathParam("id") String id);
Can't we return the single router here, or `null` if not found instead of a collection with a single element? This is how other get methods behave.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14010902
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
Swift seems to be failing. But still works on my machine. I suspect maybe this branch needs a rebase.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49259625
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + return dnsNameServers;
> + }
> +
> + /**
> + * @return Configurable maximum amount of routes per subnet. The default is 20.
> + */
> + @Nullable
> + public Set<HostRoute> getHostRoutes() {
> + return hostRoutes;
> + }
> +
> + /**
> + * @return The IP v6 Address Mode.
> + */
> + @Nullable
> + public IPv6DHCPMode getIpv6AddressMode() {
I think you have 2 different variables here: the address mode and the router advertisement. Naming could be improved though.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15362114
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
Closed #111.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#event-145797651
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #290](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/290/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-46380419
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1394](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1394/) 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-openstack/pull/111#issuecomment-48661795
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1333](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1333/) UNSTABLE
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-openstack/pull/111#issuecomment-47340581
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1431](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1431/) 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-openstack/pull/111#issuecomment-49374723
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * 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.neutron.v2.util;
> +
> +import java.lang.reflect.Field;
> +
> +public class ClassUtil {
> +
> + public static Field findField(Class clazz, String fieldName) {
This is just copied over from neutron v2_0 to ensure live tests can work the same way. It will be removed eventually, but I don't want to change it till then.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15362301
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1462](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1462/) 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-openstack/pull/111#issuecomment-50082048
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
Rebased, waiting for tests.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-50201944
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> +import java.io.Closeable;
> +import java.util.Set;
> +
> +/**
> + * Provides synchronous access to Neutron.
> + * <p/>
> + *
> + * @see <a href="http://docs.openstack.org/api/openstack-network/2.0/content/">api doc</a>
> + */
> +public interface NeutronApi extends Closeable {
> + /**
> + * @return the Zone codes configured
> + */
> + @Provides
> + @Zone
> + Set<String> getConfiguredZones();
Is there a reason NOT to switch to "Regions" now?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15358535
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + @Path("/{id}")
> + @SelectJson("network")
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + @Nullable
> + Network get(@PathParam("id") String id);
> +
> + /**
> + * Create a new network with the specified type
> + *
> + * @param network Describes the network to be created.
> + * @return a reference of the newly-created network
> + */
> + @Named("network:create")
> + @POST
> + @SelectJson("network")
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
Same as above, can this POST return a 404?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14011038
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
Very good info. Just to make sure: it uses the jclouds naming annotation strategy, right?
I will have to look a little bit more into the possible portability issues, but it makes sense! Thanks for the help.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14219150
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> +import org.jclouds.javax.annotation.Nullable;
> +import org.jclouds.location.Zone;
> +import org.jclouds.location.functions.ZoneToEndpoint;
> +import org.jclouds.openstack.neutron.v2.extensions.RouterApi;
> +import org.jclouds.openstack.neutron.v2.features.NetworkApi;
> +import org.jclouds.openstack.neutron.v2.features.PortApi;
> +import org.jclouds.openstack.neutron.v2.features.SubnetApi;
> +import org.jclouds.openstack.v2_0.features.ExtensionApi;
> +import org.jclouds.rest.annotations.Delegate;
> +import org.jclouds.rest.annotations.EndpointParam;
> +
> +import java.io.Closeable;
> +import java.util.Set;
> +
> +/**
> + * Provides synchronous access to Neutron.
"Provides synchronous access to the OpenStack Networking (Neturon) v2 API." ?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r13920897
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers list(PaginationOptions options);
> +
> + /**
> + * Returns a Routers collection that should contain a single router with the id requested.
> + *
> + * @param id the id of the router to return
> + * @return Routers collection or empty if not found
> + */
> + @Named("router:get")
> + @GET
> + @Path("/{id}")
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers get(@PathParam("id") String id);
Nice! :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14019361
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> +import javax.ws.rs.PUT;
> +import javax.ws.rs.Path;
> +import javax.ws.rs.PathParam;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * Provides synchronous access to Router operations on the OpenStack Neutron API.
> + * <p/>
> + * A logical entity for forwarding packets across internal subnets and NATting them on external
> + * networks through an appropriate external gateway.
> + *
> + * @see <a href=
> + * "http://docs.openstack.org/api/openstack-network/2.0/content/router_ext.html">api doc</a>
> + */
> +@Beta
> +@Path("/v2.0/routers")
I probably kept it because the old code had it like that. Both seem to pass live tests.
In devstack, there is no version in the endpoint path.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15361404
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * 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.neutron.v2.util;
> +
> +import java.lang.reflect.Field;
> +
> +public class ClassUtil {
> +
> + public static Field findField(Class clazz, String fieldName) {
I believe that `getField(String name)` as defined in the [Class](http://docs.oracle.com/javase/7/docs/api/java/lang/Class.html#getField\(java.lang.String\)) object would work here and you don't need the extra class.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15360111
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1324](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1324/) 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-openstack/pull/111#issuecomment-47151463
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #308](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/308/) 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-openstack/pull/111#issuecomment-48661814
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
I think so. I've played a bit more with it, replacing the `@SerializedName` annotation by `@Named` and worked too.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14293963
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> +
> + /**
> + * @return the Builder for creating a new Router
> + */
> + public static CreateBuilder createOptions(String name) {
> + return new CreateBuilder(name);
> + }
> +
> + /**
> + * @return the Builder for updating a Router
> + */
> + public static UpdateBuilder updateOptions() {
> + return new UpdateBuilder();
> + }
> +
> + private static abstract class Builder<ParametrizedBuilderType> {
Nice catch!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15358445
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1424](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1424/) 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-openstack/pull/111#issuecomment-49259824
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1334](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1334/) 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-openstack/pull/111#issuecomment-47346467
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1353](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1353/) 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-openstack/pull/111#issuecomment-47699791
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #320](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/320/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49370128
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
I am at a loss. The tests deserialize the GSON just fine, despite lacking the ConstructorProperties annotation, including in this case. AddressPair is used in CreatePort and consequently can be returned by the service and used in Port. I added a bit to the testCreatePort test just to test AddressPair is deserialized properly.
I thought https://github.com/jclouds/jclouds/blob/4c74b497547e42b8bdc94dbae3d4cd94ff3945d6/core/src/main/java/org/jclouds/json/internal/NamingStrategies.java#L160 in conjunction with the naming strategies accomplishes that. I might be wrong!
Can you elaborate with a gist of what the issue is?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14201986
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #314](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/314/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49241692
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #309](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/309/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-48666588
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + @GET
> + @Path("/{id}")
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers get(@PathParam("id") String id);
> +
> + /**
> + * Create a new router
> + *
> + * @param router Options for creating a router
> + * @return the newly created router
> + */
> + @Named("router:create")
> + @POST
> + @SelectJson("router")
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
They should be present at the interface/class level (I will double check).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14017685
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
> + this.macAddress = macAddress;
> + this.ipAddress = ipAddress;
Good catch. I will have to double check the smaller domain objects for similar cases.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14017733
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #301](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/301/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-47699884
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #316](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/316/) 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-openstack/pull/111#issuecomment-49245521
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> +import java.io.Closeable;
> +import java.util.Set;
> +
> +/**
> + * Provides synchronous access to Neutron.
> + * <p/>
> + *
> + * @see <a href="http://docs.openstack.org/api/openstack-network/2.0/content/">api doc</a>
> + */
> +public interface NeutronApi extends Closeable {
> + /**
> + * @return the Zone codes configured
> + */
> + @Provides
> + @Zone
> + Set<String> getConfiguredZones();
No, and its a good time because this is a new API. If we do it now, then deprecating the Zone concepts becomes irrelevant. One less API to deprecate/migrate to Regions SGTM! :+1:
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15373608
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
Reopened #111.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#event-142494361
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -63,7 +64,7 @@ public static Properties defaultProperties() {
>
> protected Builder() {
> super(NeutronApi.class);
> - id("openstack-neutron")
> + id("openstack-neutron-legacy")
:+1: legacy
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15359324
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #332](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/332/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-50082693
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
I tried it both ways (order of the fields) and it works. Let me update the PR.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14208145
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1423](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1423/) 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-openstack/pull/111#issuecomment-49251636
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
I had this PR in the bottom of the TODO list, sorry for my late reply!
I like the new pattern you propose. The model and the APIs look cleaner. The code look good to me, just a couple comments:
* There are several POST requests that have the "ReturnNullOn404" fallback. Does the service really return a 404 on POSTs?
* In the model, there are several classes without the `@ConstructorProperties` annotation in the constructor. Should it be added in all them?
Thanks for the refactor @zack-shoylev!`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-48399251
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> +import java.io.Closeable;
> +import java.util.Set;
> +
> +/**
> + * Provides synchronous access to Neutron.
> + * <p/>
> + *
> + * @see <a href="http://docs.openstack.org/api/openstack-network/2.0/content/">api doc</a>
> + */
> +public interface NeutronApi extends Closeable {
> + /**
> + * @return the Zone codes configured
> + */
> + @Provides
> + @Zone
> + Set<String> getConfiguredZones();
Some concerns about the Zone annotation though.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15374085
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #292](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/292/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-47137530
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #318](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/318/) 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-openstack/pull/111#issuecomment-49259881
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1291](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1291/) 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-openstack/pull/111#issuecomment-46380806
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + @GET
> + @Path("/{id}")
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers get(@PathParam("id") String id);
> +
> + /**
> + * Create a new router
> + *
> + * @param router Options for creating a router
> + * @return the newly created router
> + */
> + @Named("router:create")
> + @POST
> + @SelectJson("router")
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
Do we know if this POST can return a 404?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14679790
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1429](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1429/) 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-openstack/pull/111#issuecomment-49369974
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers list(PaginationOptions options);
> +
> + /**
> + * Returns a Routers collection that should contain a single router with the id requested.
> + *
> + * @param id the id of the router to return
> + * @return Routers collection or empty if not found
> + */
> + @Named("router:get")
> + @GET
> + @Path("/{id}")
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers get(@PathParam("id") String id);
No, because of the array. Check the test JSON file - https://github.com/rackerlabs/jclouds-labs-openstack/blob/neutron-refactoring/openstack-neutron/src/test/resources/router_get_response.json
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14019138
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #295](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/295/) 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-openstack/pull/111#issuecomment-47291044
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
The documentation for this will probably keep getting updated. But this should make it in 1.8.
Note: examples will have to be updated/created.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49950900
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> + * under the License.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +import java.beans.ConstructorProperties;
> +
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
I thought we were going to use `@Name` rather `@SerializedName`. No?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15237013
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> +import java.io.Closeable;
> +import java.util.Set;
> +
> +/**
> + * Provides synchronous access to Neutron.
> + * <p/>
> + *
> + * @see <a href="http://docs.openstack.org/api/openstack-network/2.0/content/">api doc</a>
> + */
> +public interface NeutronApi extends Closeable {
> + /**
> + * @return the Zone codes configured
> + */
> + @Provides
> + @Zone
> + Set<String> getConfiguredZones();
I wonder... Should we use `getConfiguredRegions()` now in an effort to move the OpenStack APIs to use regions? Its one less change to do in the future. WDYT? cc/ @everett-toews
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15356596
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #335](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/335/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-50203344
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
> + this.macAddress = macAddress;
> + this.ipAddress = ipAddress;
Also add null checks for all mandatory properties.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14010798
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
Reopened #111.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#event-142465954
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers list(PaginationOptions options);
> +
> + /**
> + * Returns a Routers collection that should contain a single router with the id requested.
> + *
> + * @param id the id of the router to return
> + * @return Routers collection or empty if not found
> + */
> + @Named("router:get")
> + @GET
> + @Path("/{id}")
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers get(@PathParam("id") String id);
At this point I am waiting on some feedback and will update here when I get it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14022962
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1432](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1432/) 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-openstack/pull/111#issuecomment-49378702
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #293](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/293/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-47151258
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
Add the `@ConstructorProperties` annotation here and in all other domain objects, for consistency?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14010794
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1403](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1403/) 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-openstack/pull/111#issuecomment-48980854
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1421](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1421/) 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-openstack/pull/111#issuecomment-49245389
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
Hi reviewers!
There were a number of concerns with how the domain objects were implemented in the old neutron and some different concerns with the refactored code.
Problems with the old code:
Options and Mapbinders (extra code, doesn't utilize GSON enough).
Newer code:
Create/Update/List objects were confusing.
Latest code:
I added some changes to the Router, RouterApi, and RouterApiMockTest
RouterApi
https://github.com/rackerlabs/jclouds-labs-openstack/blob/neutron-refactoring/openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/extensions/RouterApi.java
Router
https://github.com/rackerlabs/jclouds-labs-openstack/blob/neutron-refactoring/openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/Router.java
Tests
https://github.com/rackerlabs/jclouds-labs-openstack/blob/neutron-refactoring/openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2/features/RouterApiMockTest.java
Main points improved in the Router refactoring:
You use createOptions or updateOptions; you can only pass createOptions.build() to the create method; you can only pass updateOptions.build to the update method. You get Router when listing, so you cannot pass it back to the service.
You have to use createOptions or updateOptions to instantiate an object as a user.
These have different validation paths.
They can require different parameters to be present (in this example createOptions requires name and adminStateUp).
This also ensures code is relatively dry (attributes of the domain object are not repeated and neither is code). This should also keep the domain objects immutable as before.
This seems to be a good pattern going forward for openstack, or at least neutron domain objects. Feedback will be much appreciated.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-47837869
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
When deserializing, as there is no default constructor, we need to give gson some hints about which values have to be passed to each constructor argument. This is done with the `@ConstructorProperties` annotation.
What I tried to say is that, even if the AddressPair class is not being used in deserialization (because it is an entity that might not be returned by the provider API), I'd rather add the annotation to the constructor, to complete the deserialization "configuration". Having the fields annotated is not enough if there is no default constructor.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14193425
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Everett Toews <no...@github.com>.
That's what I did previously (use the admin account).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49755250
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -0,0 +1,66 @@
> +/**
Is this class even necessary if we are using MWS tests?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15359627
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers list(PaginationOptions options);
> +
> + /**
> + * Returns a Routers collection that should contain a single router with the id requested.
> + *
> + * @param id the id of the router to return
> + * @return Routers collection or empty if not found
> + */
> + @Named("router:get")
> + @GET
> + @Path("/{id}")
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers get(@PathParam("id") String id);
Well I did some digging!
I can combine:
@SelectJson("routers")
@OnlyElement
And that will work. I still need to figure out why the service does this.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14019261
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
I will be rebasing and merging later today or tomorrow in preparation for next week's release, unless there are other comments.
Unless critical, such might have to be addressed in further PRs (this one is big enough as it is).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-50079422
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers list(PaginationOptions options);
> +
> + /**
> + * Returns a Routers collection that should contain a single router with the id requested.
> + *
> + * @param id the id of the router to return
> + * @return Routers collection or empty if not found
> + */
> + @Named("router:get")
> + @GET
> + @Path("/{id}")
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers get(@PathParam("id") String id);
For some reason the call returns a collection. I am not completely sure yet as to why. It could be just a neutron weirdness.
Is there a simple way to do this with a jclouds annotation?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14017582
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> + }
> +
> + for(Subnet subnet : subnetApi.list().concat().toList()) {
> + subnetApi.delete(subnet.getId());
> + }
> +
> + for(Port port : portApi.list().concat().toList()) {
> + portApi.delete(port.getId());
> + }
> +
> + for(Network network : networkApi.list().concat().toList()) {
> + if("private".equals(network.getName()) || "public".equals(network.getName()))continue;
> + networkApi.delete(network.getId());
> + }
> + }
> + }*/
Is there a reason this is disabled?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15359366
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
Yes, I also tried it locally. Now I'm playing with some other changes, just to completely understand how the deserialization stuff works :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14210097
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
Based on the documentation I am pretty sure that /v2.0/ is not part of what is returned by the endpoint - for whatever reason.
For example, the current specs
https://wiki.openstack.org/wiki/Neutron/APIv2-specification
have all requests explicitly with the version in the path.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-50056411
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> +
> + Exception exception = message != null ? new HttpResponseException(command, response, message)
> + : new HttpResponseException(command, response);
> + message = message != null ? message : String.format("%s -> %s", command.getCurrentRequest().getRequestLine(),
> + response.getStatusLine());
> + switch (response.getStatusCode()) {
> + case 400:
> + break;
> + case 401:
> + case 403:
> + exception = new AuthorizationException(message, exception);
> + break;
> + case 404:
> + if (!command.getCurrentRequest().getMethod().equals("DELETE")) {
> + exception = new ResourceNotFoundException(message, exception);
> + }
I'll add it.
Note this was not in the old code.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15361833
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
Reopened #111.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#event-135878223
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1364](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1364/) 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-openstack/pull/111#issuecomment-47836903
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
@zack-shoylev Just a couple of nits here and there, but its looking great! If you could:
- run all of the JSON resources through [JSONLint](jsonlint.org).
- determine if the endpoint needs the `v2.0` in the service catalog and update all references.
... that would be awesome. Thx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-50051026
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
merged
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-50206106
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> + public static Properties defaultProperties() {
> + Properties properties = BaseHttpApiMetadata.defaultProperties();
> + properties.setProperty(SERVICE_TYPE, ServiceType.NETWORK);
> + properties.setProperty(CREDENTIAL_TYPE, CredentialTypes.PASSWORD_CREDENTIALS);
> + return properties;
> + }
> +
> + public static class Builder extends BaseHttpApiMetadata.Builder<NeutronApi, Builder> {
> +
> + protected Builder() {
> + super(NeutronApi.class);
> + id("openstack-neutron")
> + .name("OpenStack Neutron API")
> + .identityName("${tenantName}:${userName} or ${userName}, if your keystone supports a default tenant")
> + .credentialName("${password}")
> + .endpointName("KeyStone base url ending in /v2.0/")
"Keystone"
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15236906
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1430](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1430/) 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-openstack/pull/111#issuecomment-49372386
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
Closed #111.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#event-142494359
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> +import org.jclouds.openstack.neutron.v2.extensions.RouterApi;
> +import org.jclouds.openstack.neutron.v2.features.NetworkApi;
> +import org.jclouds.openstack.neutron.v2.features.PortApi;
> +import org.jclouds.openstack.neutron.v2.features.SubnetApi;
> +import org.jclouds.openstack.v2_0.features.ExtensionApi;
> +import org.jclouds.rest.annotations.Delegate;
> +import org.jclouds.rest.annotations.EndpointParam;
> +
> +import java.io.Closeable;
> +import java.util.Set;
> +
> +/**
> + * Provides synchronous access to Neutron.
> + * <p/>
> + *
> + * @see <a href="http://docs.openstack.org/api/openstack-network/2.0/content/">api doc</a>
Please remove the external link to API docs
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r13920819
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
Thanks so much for having a look!
ReturnNullOn404 - this needs to be fixed (I am pretty sure service does not return it, but still double checking some sources)
@ConstructorProperties - based on previous discussions, I will add it back to domain classes. The plan is to rework those to follow the Router model, and ConstructorProperties is part of it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-48401994
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + }
> +
> + for(Subnet subnet : subnetApi.list().concat().toList()) {
> + subnetApi.delete(subnet.getId());
> + }
> +
> + for(Port port : portApi.list().concat().toList()) {
> + portApi.delete(port.getId());
> + }
> +
> + for(Network network : networkApi.list().concat().toList()) {
> + if("private".equals(network.getName()) || "public".equals(network.getName()))continue;
> + networkApi.delete(network.getId());
> + }
> + }
> + }*/
The new live tests pretty much are a 1:1 rewrite of the old neutron tests. Unfortunately, they might need to be refactored more to use proper setup/teardown - in some cases, especially failures, you will have left-over entities. Running this test is a nice way to cleanup your account, but doesn't make sense leaving it enabled. It might make sense to control it through some sort of property instead of just commenting it out, though. Best solution would be to refactor the tests though.
Thoughts?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15359786
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #327](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/327/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49950911
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
Closed #111.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#event-142465951
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #296](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/296/) 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-openstack/pull/111#issuecomment-47340746
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> + * doc</a>
> + */
> +public class Network {
> +
> + private String id;
> + private NetworkStatus status;
> + private Set<String> subnets;
> +
> + private String name;
> + @Named("admin_state_up")
> + private Boolean adminStateUp;
> + private Boolean shared;
> + @Named("tenant_id")
> + private String tenantId;
> +
> + // providernet.py: Provider Networks Extension
There has to be a better way to handle these extension fields! I just don't know what it is yet... Thanks for commenting on the actual Neutron extension, however I think that flipping the comment order will be easier to read:
`// Provider Networks Extension: providernet.py`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15357317
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
Actually ConstructorProperties leads to problems when updating the code and is also unnecessary when SerializedName is used. I have removed it completely, except from the simplest cases. For consistency, I could remove it from the rest of the code.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14017479
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #323](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/323/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49378815
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> +import javax.ws.rs.Produces;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * Provides synchronous access to Network operations on the openstack Neutron API.
> + * <p/>
> + * Each tenant can define one or more networks. A network is a virtual isolated layer-2 broadcast domain reserved to the
> + * tenant. A tenant can create several ports for a network, and plug virtual interfaces into these ports.
> + *
> + * @see <a href=
> + * "http://docs.openstack.org/api/openstack-network/2.0/content/Networks.html">api doc</a>
> + */
> +@Path("/v2.0/networks")
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Produces(MediaType.APPLICATION_JSON)
Consider moving this annotation to those methods that generate a body.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14011016
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
Alright, live tests now pass as well!
However, testing on devstack-icehouse, the following changes have to be made to the neutron policy.json (/opt/stack/neutron/etc/policy.json):
1. create_network:router:external
2. get_network:provider:network_type
3. create_network:provider:network_type
4. update_network:provider:network_type
should not be be rule:admin_only (leave empty instead).
Run:
mvn -Plive clean install "-Dtest.openstack-neutron-legacy.endpoint=http://localhost:5000/v2.0" "-Dtest.openstack-neutron.endpoint=http://localhost:5000/v2.0" "-Dtest.openstack-neutron.identity=demo:demo" "-Dtest.openstack-neutron-
legacy.identity=demo:demo" "-Dtest.openstack-neutron.credential=devstack" "-Dtest.openstack-neutron-legacy.credential=devstack"
Results:
Starting test testBulkCreateSubnet(org.jclouds.openstack.neutron.v2_0.features.SubnetApiLiveTest)
Starting test testBulkCreateSubnet(org.jclouds.openstack.neutron.v2.features.SubnetApiLiveTest)
[pool-1-thread-1] Test testBulkCreateSubnet(org.jclouds.openstack.neutron.v2.features.SubnetApiLiveTest) succeeded: 1962ms
Test suite progress: tests succeeded: 1, failed: 0, skipped: 0.
Starting test testCreateUpdateAndDeleteSubnet(org.jclouds.openstack.neutron.v2.features.SubnetApiLiveTest)
[pool-1-thread-2] Test testBulkCreateSubnet(org.jclouds.openstack.neutron.v2_0.features.SubnetApiLiveTest) succeeded: 2063ms
Test suite progress: tests succeeded: 2, failed: 0, skipped: 0.
Starting test testCreateUpdateAndDeleteSubnet(org.jclouds.openstack.neutron.v2_0.features.SubnetApiLiveTest)
[pool-1-thread-1] Test testCreateUpdateAndDeleteSubnet(org.jclouds.openstack.neutron.v2.features.SubnetApiLiveTest) succeeded: 1147ms
Test suite progress: tests succeeded: 3, failed: 0, skipped: 0.
[pool-1-thread-2] Test testCreateUpdateAndDeleteSubnet(org.jclouds.openstack.neutron.v2_0.features.SubnetApiLiveTest) succeeded: 1058ms
Test suite progress: tests succeeded: 4, failed: 0, skipped: 0.
Starting test testGetAndListSubnets(org.jclouds.openstack.neutron.v2_0.features.SubnetApiLiveTest)
[pool-1-thread-2] Test testGetAndListSubnets(org.jclouds.openstack.neutron.v2_0.features.SubnetApiLiveTest) succeeded: 164ms
Test suite progress: tests succeeded: 5, failed: 0, skipped: 0.
Starting test testCreateAndDeleteRouterInterfaceForPort(org.jclouds.openstack.neutron.v2_0.extensions.RouterApiLiveTest)
Starting test testCreateAndDeleteRouterInterfaceForPort(org.jclouds.openstack.neutron.v2.features.RouterApiLiveTest)
[pool-6-thread-1] Test testCreateAndDeleteRouterInterfaceForPort(org.jclouds.openstack.neutron.v2.features.RouterApiLiveTest) succeeded: 3330ms
Test suite progress: tests succeeded: 6, failed: 0, skipped: 0.
Starting test testCreateAndDeleteRouterInterfaceForSubnet(org.jclouds.openstack.neutron.v2.features.RouterApiLiveTest)
[pool-6-thread-2] Test testCreateAndDeleteRouterInterfaceForPort(org.jclouds.openstack.neutron.v2_0.extensions.RouterApiLiveTest) succeeded: 3437ms
Test suite progress: tests succeeded: 7, failed: 0, skipped: 0.
Starting test testCreateAndDeleteRouterInterfaceForSubnet(org.jclouds.openstack.neutron.v2_0.extensions.RouterApiLiveTest)
[pool-6-thread-1] Test testCreateAndDeleteRouterInterfaceForSubnet(org.jclouds.openstack.neutron.v2.features.RouterApiLiveTest) succeeded: 2663ms
Test suite progress: tests succeeded: 8, failed: 0, skipped: 0.
Starting test testCreateUpdateAndDeleteRouter(org.jclouds.openstack.neutron.v2.features.RouterApiLiveTest)
[pool-6-thread-2] Test testCreateAndDeleteRouterInterfaceForSubnet(org.jclouds.openstack.neutron.v2_0.extensions.RouterApiLiveTest) succeeded: 2871ms
Test suite progress: tests succeeded: 9, failed: 0, skipped: 0.
Starting test testCreateUpdateAndDeleteRouter(org.jclouds.openstack.neutron.v2_0.extensions.RouterApiLiveTest)
[pool-6-thread-1] Test testCreateUpdateAndDeleteRouter(org.jclouds.openstack.neutron.v2.features.RouterApiLiveTest) succeeded: 1605ms
Test suite progress: tests succeeded: 10, failed: 0, skipped: 0.
[pool-6-thread-2] Test testCreateUpdateAndDeleteRouter(org.jclouds.openstack.neutron.v2_0.extensions.RouterApiLiveTest) succeeded: 1574ms
Test suite progress: tests succeeded: 11, failed: 0, skipped: 0.
Starting test testGetAndListRouters(org.jclouds.openstack.neutron.v2_0.extensions.RouterApiLiveTest)
[pool-6-thread-2] Test testGetAndListRouters(org.jclouds.openstack.neutron.v2_0.extensions.RouterApiLiveTest) succeeded: 142ms
Test suite progress: tests succeeded: 12, failed: 0, skipped: 0.
Starting test testBulkCreatePort(org.jclouds.openstack.neutron.v2.features.PortApiLiveTest)
Starting test testBulkCreatePort(org.jclouds.openstack.neutron.v2_0.features.PortApiLiveTest)
[pool-11-thread-2] Test testBulkCreatePort(org.jclouds.openstack.neutron.v2_0.features.PortApiLiveTest) succeeded: 3753ms
Test suite progress: tests succeeded: 13, failed: 0, skipped: 0.
Starting test testCreateUpdateAndDeletePort(org.jclouds.openstack.neutron.v2_0.features.PortApiLiveTest)
[pool-11-thread-1] Test testBulkCreatePort(org.jclouds.openstack.neutron.v2.features.PortApiLiveTest) succeeded: 3876ms
Test suite progress: tests succeeded: 14, failed: 0, skipped: 0.
Starting test testCreateUpdateAndDeletePort(org.jclouds.openstack.neutron.v2.features.PortApiLiveTest)
[pool-11-thread-2] Test testCreateUpdateAndDeletePort(org.jclouds.openstack.neutron.v2_0.features.PortApiLiveTest) succeeded: 2537ms
Test suite progress: tests succeeded: 15, failed: 0, skipped: 0.
Starting test testGetAndListPorts(org.jclouds.openstack.neutron.v2_0.features.PortApiLiveTest)
[pool-11-thread-2] Test testGetAndListPorts(org.jclouds.openstack.neutron.v2_0.features.PortApiLiveTest) succeeded: 73ms
Test suite progress: tests succeeded: 16, failed: 0, skipped: 0.
[pool-11-thread-1] Test testCreateUpdateAndDeletePort(org.jclouds.openstack.neutron.v2.features.PortApiLiveTest) succeeded: 2573ms
Test suite progress: tests succeeded: 17, failed: 0, skipped: 0.
Starting test testBulkCreateNetwork(org.jclouds.openstack.neutron.v2_0.features.NetworkApiLiveTest)
Starting test testBulkCreateNetwork(org.jclouds.openstack.neutron.v2.features.NetworkApiLiveTest)
[pool-16-thread-1] Test testBulkCreateNetwork(org.jclouds.openstack.neutron.v2.features.NetworkApiLiveTest) succeeded: 1297ms
Test suite progress: tests succeeded: 18, failed: 0, skipped: 0.
Starting test testCreateUpdateAndDeleteNetwork(org.jclouds.openstack.neutron.v2.features.NetworkApiLiveTest)
[pool-16-thread-2] Test testBulkCreateNetwork(org.jclouds.openstack.neutron.v2_0.features.NetworkApiLiveTest) succeeded: 1318ms
Test suite progress: tests succeeded: 19, failed: 0, skipped: 0.
Starting test testCreateUpdateAndDeleteNetwork(org.jclouds.openstack.neutron.v2_0.features.NetworkApiLiveTest)
[pool-16-thread-2] Test testCreateUpdateAndDeleteNetwork(org.jclouds.openstack.neutron.v2_0.features.NetworkApiLiveTest) succeeded: 1023ms
Test suite progress: tests succeeded: 20, failed: 0, skipped: 0.
Starting test testGetAndListNetworks(org.jclouds.openstack.neutron.v2_0.features.NetworkApiLiveTest)
[pool-16-thread-1] Test testCreateUpdateAndDeleteNetwork(org.jclouds.openstack.neutron.v2.features.NetworkApiLiveTest) succeeded: 1184ms
Test suite progress: tests succeeded: 21, failed: 0, skipped: 0.
[pool-16-thread-2] Test testGetAndListNetworks(org.jclouds.openstack.neutron.v2_0.features.NetworkApiLiveTest) succeeded: 220ms
Test suite progress: tests succeeded: 22, failed: 0, skipped: 0.
Tests run: 22, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 23.567 sec - in TestSuite
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49391829
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + @GET
> + @Path("/{id}")
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers get(@PathParam("id") String id);
> +
> + /**
> + * Create a new router
> + *
> + * @param router Options for creating a router
> + * @return the newly created router
> + */
> + @Named("router:create")
> + @POST
> + @SelectJson("router")
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
Is a `@Produces` annotation be needed here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14010985
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1408](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1408/) 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-openstack/pull/111#issuecomment-49105634
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> +
> + /**
> + * @return the Builder for creating a new Router
> + */
> + public static CreateBuilder createOptions(String name) {
> + return new CreateBuilder(name);
> + }
> +
> + /**
> + * @return the Builder for updating a Router
> + */
> + public static UpdateBuilder updateOptions() {
> + return new UpdateBuilder();
> + }
> +
> + private static abstract class Builder<ParametrizedBuilderType> {
Idea was saying Parameterized is wrong. Checked merriam-webster's and it seems both are correct.
http://www.merriam-webster.com/dictionary/parameterize
Probably still going to fix it though.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15359259
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
The annotation is needed if the AddressPair object is at some point returned by the api and has to be deserialized.
Even when it is not the case, I tend to prefer being consistent and add the "deserialization hints" whenever we add the "serialization ones". But it is just a matter of completeness and a personal feeling :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14018380
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #325](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/325/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49392398
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #304](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/304/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-47840000
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1434](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1434/) 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-openstack/pull/111#issuecomment-49392408
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #298](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/298/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-47348972
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1470](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1470/) 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-openstack/pull/111#issuecomment-50203112
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1469](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1469/) 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-openstack/pull/111#issuecomment-50189339
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
Mystery solved :)
What made me doubt is the fact that to deserialize the JSON, if there is no default constructor, the one with parameters must be used and Gson must know in which order the values must be passed. It is not an option to pass null values and set them afterward by reflection, because the constructor can have null check validations.
I've debugged the code, and Gson, when the `@ConstructorProperties` annotation is not found, picks an internal implementation that ends up using their [UnsafeAllocator](https://code.google.com/p/google-gson/source/browse/tags/gson-2.2.4/src/main/java/com/google/gson/internal/UnsafeAllocator.java), which uses `sun.misc.Unsafe` to directly allocate a new instance of the deserialization class in memory without calling the constructor. After this, it sets the field with reflection, according to the field annotations.
Although this works, it has several issues: the constructor might not be used at all and all validations in there may not be triggered, and also using `sun.misc.Unsafe` might not work in all implementations of the JVM.
I think we should try to be as portable as possible, so I'd recommend adding the annotation, now we know more about the deserialization internals. WDYT?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14213281
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
+1 to the changes we discussed yesterday. Rebase/squash and let's push it!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-50200988
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
This is missing:
- Better documentation
- Live tests
I would like to get feedback on the refactoring part. It is my understanding this approach is how we want things going forward - let me know if this is not the case.
This is mostly based on an email with a refactoring proposal for neutron I sent a while back to the dev list. (http://mail-archives.apache.org/mod_mbox/jclouds-dev/201403.mbox/%3C3918EC648ABC75468E8516B58EB8169C01802ADB@ORD1EXD04.RACKSPACE.CORP%3E)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-46380375
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * A Neutron Subnet Allocation Pool
> + *
> + * @see <a
> + * href="http://docs.openstack.org/api/openstack-network/2.0/content/Subnets.html">api
> + * doc</a>
> + */
> +public class AllocationPool {
> +
> + protected final String start;
> + protected final String end;
> +
> + protected AllocationPool(String start, String end) {
Does this need the `@ConstructorProperties` annotation?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14679656
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers list(PaginationOptions options);
> +
> + /**
> + * Returns a Routers collection that should contain a single router with the id requested.
> + *
> + * @param id the id of the router to return
> + * @return Routers collection or empty if not found
> + */
> + @Named("router:get")
> + @GET
> + @Path("/{id}")
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers get(@PathParam("id") String id);
May [Unwrap](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/rest/annotations/Unwrap.java) or [SelectJson](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/rest/annotations/SelectJson.java) work?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14018579
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #321](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/321/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49372397
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> + @SelectJson("router")
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + @Nullable
> + Router update(@PathParam("id") String id, @WrapWith("router") Router.UpdateOptions router);
> +
> + /**
> + * Deletes the specified router
> + *
> + * @param id the id of the router to delete
> + * @return true if delete successful, false if not
> + */
> + @Named("router:delete")
> + @DELETE
> + @Path("/{id}")
> + @Fallback(Fallbacks.FalseOnNotFoundOr404.class)
> + boolean delete(@PathParam("id") String id);
Are we going with primitive `boolean` or `Boolean` in the APIs?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15358999
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1457](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1457/) 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-openstack/pull/111#issuecomment-50058406
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
Rebased. Should be ready for another round of reviews. Fully featured.
Some suggestions:
1. Use @Beta ? Yes/no?
2. More documentation. Challange: A lot of the new stuff has little to no documentation in the API docs.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49392103
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> +import javax.ws.rs.PUT;
> +import javax.ws.rs.Path;
> +import javax.ws.rs.PathParam;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * Provides synchronous access to Router operations on the OpenStack Neutron API.
> + * <p/>
> + * A logical entity for forwarding packets across internal subnets and NATting them on external
> + * networks through an appropriate external gateway.
> + *
> + * @see <a href=
> + * "http://docs.openstack.org/api/openstack-network/2.0/content/router_ext.html">api doc</a>
> + */
> +@Beta
> +@Path("/v2.0/routers")
Using `v2.0` here is brittle. I would *assume* that it would already be appended to the endpoint's `publicURL`. What does the actual service catalog return for the Neutron endpoint? I would expect this to be `@Path("/routers")`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15358811
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #322](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/322/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49374819
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> + {
> + "publicURL":"URL/v2/da0d12be20394afb851716e10a49e4a7",
> + "id":"2122bcaa704343c19ad2578410d4961d",
> + "internalURL":"URL/v2/da0d12be20394afb851716e10a49e4a7",
> + "region":"RegionOne",
> + "adminURL":"URL/v2/da0d12be20394afb851716e10a49e4a7"
> + }
> + ]
> + },
> + {
> + "name":"neutron",
> + "type":"network",
> + "endpoints_links": [],
> + "endpoints": [
> + {
> + "publicURL":"URL/",
As mentioned, I believe that this *should* be ` "publicURL":"URL/v2.0"`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15360186
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #334](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/334/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-50189676
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> +
> + /**
> + * @return the Builder for creating a new Router
> + */
> + public static CreateBuilder createOptions(String name) {
> + return new CreateBuilder(name);
> + }
> +
> + /**
> + * @return the Builder for updating a Router
> + */
> + public static UpdateBuilder updateOptions() {
> + return new UpdateBuilder();
> + }
> +
> + private static abstract class Builder<ParametrizedBuilderType> {
The spelling of `ParametrizedBuilderType` is incorrect and should be `ParameterizedBuilderType`. Missing the 'e' !
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15357429
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Everett Toews <no...@github.com>.
@zack-shoylev @jdaggett @ccustine
I started a new wiki page [OpenStack Testing](https://wiki.apache.org/jclouds/Testing%20OpenStack) to capture the how-to info you provided Zack. Let's continue to expand on it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49457445
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #310](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/310/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-48980938
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1331](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1331/) UNSTABLE
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-openstack/pull/111#issuecomment-47291430
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
The JSON deserialization policy is configured [here](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/json/config/GsonModule.java#L121-L125) in the `GsonModule`. If you have a look at [how the registered type adapter factory](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/json/internal/DeserializationConstructorAndReflectiveTypeAdapterFactory.java#L123-L128) works, you'll see that if the `@ConstructorProperties` annotation is not present, it returns `null` and lets Gson choose the right type adapter. Otherwise the jclouds custom one is used. The latter will take into account the annotation values to pass the right values to each constructor argument, but given the results of your tests I don't really know what the former will do :) We're learning new stuff today.
Could you modify the JSON in your test to have the `ip_address` first and then the `mac_address` in the `allowed_address_pairs`list? I would be surprised if the test still succeeds, as that would mean that Gson is smart enough to know which value it has to pass to each constructor argument, without any hint.
The way I understand it, if you change the JSON this way, the test will fail, and if you add the `@ConstructorProperties` annotation it will succeed regardless of the order in which the JSON parameters come.
Could you change the just the order of the fields in the JSON and share the results of the test?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14206804
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1365](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1365/) 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-openstack/pull/111#issuecomment-47839808
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + @Path("/{id}")
> + @SelectJson("network")
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + @Nullable
> + Network get(@PathParam("id") String id);
> +
> + /**
> + * Create a new network with the specified type
> + *
> + * @param network Describes the network to be created.
> + * @return a reference of the newly-created network
> + */
> + @Named("network:create")
> + @POST
> + @SelectJson("network")
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
This is a good question, I will have to double check some source codes.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14017389
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #297](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/297/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-47346308
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + @Named("router:create")
> + @POST
> + @SelectJson("router")
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + @Nullable
> + Router create(@WrapWith("router") CreateRouter router);
> +
> + /**
> + * Update a router
> + *
> + * @param id the id of the router to update
> + * @param router Contains only the attributes to update
> + * @return The modified router
> + */
> + @Named("router:update")
> + @PUT
Is a `@Produces` annotation be needed here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14010996
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1323](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1323/) 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-openstack/pull/111#issuecomment-47137474
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1395](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1395/) 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-openstack/pull/111#issuecomment-48666574
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #324](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/324/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49391952
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> + @Path("/{id}/add_router_interface")
> + @MapBinder(EmptyOptions.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + @Nullable
> + RouterInterface addInterfaceForSubnet(@PathParam("id") String routerId, @PayloadParam("subnet_id") String subnetId);
> +
> + /**
> + * Add a interface to a router to connect to the specified port
> + *
> + * @param routerId the id of the router to create the interface at
> + * @param portId the id of the port to connect with the interface
> + * @return the newly-created router interface
> + */
> + @Named("router:addInterfaceForPort")
> + @PUT
> + @Path("{id}/add_router_interface")
`@Path("/{id}/add_router_interface")` Missing the leading `/`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15358904
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #317](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/317/) 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-openstack/pull/111#issuecomment-49251704
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * A Neutron Subnet Allocation Pool
> + *
> + * @see <a
> + * href="http://docs.openstack.org/api/openstack-network/2.0/content/Subnets.html">api
> + * doc</a>
> + */
> +public class AllocationPool {
> +
> + protected final String start;
> + protected final String end;
> +
> + protected AllocationPool(String start, String end) {
Yep, will be adding it back in as part of the complete refactoring.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14684855
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + @GET
> + @Path("/{id}")
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers get(@PathParam("id") String id);
> +
> + /**
> + * Create a new router
> + *
> + * @param router Options for creating a router
> + * @return the newly created router
> + */
> + @Named("router:create")
> + @POST
> + @SelectJson("router")
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
I think it's just safer to keep them around in general. In a most cases the service will just expect them.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14017662
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
@everett-toews Seems like a good start. I spent some time trying to figure out if some of that can be automated during testing, but didn't look too promising. My other idea is trying to use an admin account during testing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49732578
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
@nacx Can you give this another look? Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-48395860
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> + @Named("router:removeInterfaceForSubnet")
> + @PUT
> + @Path("/{id}/remove_router_interface")
> + @MapBinder(EmptyOptions.class)
> + @Fallback(Fallbacks.FalseOnNotFoundOr404.class)
> + boolean removeInterfaceForSubnet(@PathParam("id") String routerId, @PayloadParam("subnet_id") String subnetId);
> +
> + /**
> + * Remove the interface where the specified port is connected to
> + *
> + * @param routerId the id of the router to remove the interface from
> + * @param portId the id of the port to disconnect from the interface
> + */
> + @Named("router:removeInterfaceForPort")
> + @PUT
> + @Path("{id}/remove_router_interface")
Leading `/` as mentioned previously
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15358929
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #303](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/303/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-47837437
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1418](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1418/) 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-openstack/pull/111#issuecomment-49241557
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + @SerializedName("n1kv:segment_add")
> + protected final String segmentAdd;
> + @SerializedName("n1kv:segment_del")
> + protected final String segmentDel;
> + @SerializedName("n1kv:member_segments")
> + protected final String memberSegments;
> +
> + // multiprovidernet.py: Multiprovider net extension; Segments and provider
> + // values cannot both be set.
> + protected final Set<NetworkSegment> segments;
> +
> + // flavor.py: Flavor support for network and router
> + @SerializedName("flavor:network")
> + protected final String networkFlavor;
> +
> + protected CreateNetwork(String name, Boolean adminStateUp, Boolean shared, String tenantId, NetworkType networkType,
Same here, does this need the `@ConstructorProperties` annotation?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14679677
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1433](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1433/) 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-openstack/pull/111#issuecomment-49391992
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + * under the License.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +import java.beans.ConstructorProperties;
> +
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
Yep, I will update for that.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15263065
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
Please keep me posted :) this stuff is not documented much.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14211225
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> + */
> +package org.jclouds.openstack.neutron.v2.domain;
> +
> +import com.google.common.base.Objects;
> +import com.google.gson.annotations.SerializedName;
> +
> +/**
> + */
> +public class AddressPair {
> +
> + @SerializedName("mac_address")
> + protected final String macAddress;
> + @SerializedName("ip_address")
> + protected final String ipAddress;
> +
> + protected AddressPair(String macAddress, String ipAddress) {
SerializedName is used for both serialization and deserialization.
When you say you prefer it for consistency, can you elaborate? Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14071020
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #312](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/312/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-49105670
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
About to rebase and get it ready for merge.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-50201613
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #329](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/329/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-50058851
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> + return dnsNameServers;
> + }
> +
> + /**
> + * @return Configurable maximum amount of routes per subnet. The default is 20.
> + */
> + @Nullable
> + public Set<HostRoute> getHostRoutes() {
> + return hostRoutes;
> + }
> +
> + /**
> + * @return The IP v6 Address Mode.
> + */
> + @Nullable
> + public IPv6DHCPMode getIpv6AddressMode() {
`getIPv6DHCPMode()`? I thought that we always use uppercase for acronyms. I could be wrong...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15357846
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Ignasi Barrera <no...@github.com>.
> + @GET
> + @Path("/{id}")
> + @ResponseParser(ParseRouters.class)
> + @Fallback(EmptyRoutersFallback.class)
> + Routers get(@PathParam("id") String id);
> +
> + /**
> + * Create a new router
> + *
> + * @param router Options for creating a router
> + * @return the newly created router
> + */
> + @Named("router:create")
> + @POST
> + @SelectJson("router")
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
Can a POST to /routers return a 404?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r14010939
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Zack Shoylev <no...@github.com>.
> +import java.io.Closeable;
> +import java.util.Set;
> +
> +/**
> + * Provides synchronous access to Neutron.
> + * <p/>
> + *
> + * @see <a href="http://docs.openstack.org/api/openstack-network/2.0/content/">api doc</a>
> + */
> +public interface NeutronApi extends Closeable {
> + /**
> + * @return the Zone codes configured
> + */
> + @Provides
> + @Zone
> + Set<String> getConfiguredZones();
Alright, will update :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15373747
Re: [jclouds-labs-openstack] Neutron Refactoring (#111)
Posted by Jeremy Daggett <no...@github.com>.
> +
> + Exception exception = message != null ? new HttpResponseException(command, response, message)
> + : new HttpResponseException(command, response);
> + message = message != null ? message : String.format("%s -> %s", command.getCurrentRequest().getRequestLine(),
> + response.getStatusLine());
> + switch (response.getStatusCode()) {
> + case 400:
> + break;
> + case 401:
> + case 403:
> + exception = new AuthorizationException(message, exception);
> + break;
> + case 404:
> + if (!command.getCurrentRequest().getMethod().equals("DELETE")) {
> + exception = new ResourceNotFoundException(message, exception);
> + }
```
case 409:
exception = new IllegalStateException(exception.getMessage(), exception);
break;
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/111/files#r15359280