Daniel Erez has posted comments on this change. Change subject: restapi: CRUD for OpenStackVolume authentication keys ......................................................................
Patch Set 6: (9 comments) https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/OpenstackVolumeAuthenticationKeyUsageType.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/OpenstackVolumeAuthenticationKeyUsageType.java: Line 11: public static OpenstackVolumeAuthenticationKeyUsageType fromValue(String v) { Line 12: try { Line 13: return valueOf(v.toUpperCase()); Line 14: } catch (IllegalArgumentException e) { Line 15: return null; > Consider sending an error message to the log describing this problem, somet Done Line 16: } Line 17: } https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/openstack/OpenStackVolumeAuthenticationKeyResource.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/openstack/OpenStackVolumeAuthenticationKeyResource.java: Line 26: Line 27: @Produces({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) Line 28: public interface OpenStackVolumeAuthenticationKeyResource { Line 29: @GET Line 30: public OpenstackVolumeAuthenticationKey get(); > The @DELETE method needs to go here, because of Resteasy 3 requirements: Done Line 31: Line 32: @PUT Line 33: @Consumes({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) Line 34: public OpenstackVolumeAuthenticationKey update(OpenstackVolumeAuthenticationKey resource); https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/openstack/OpenStackVolumeAuthenticationKeysResource.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/openstack/OpenStackVolumeAuthenticationKeysResource.java: Line 40: public Response add(OpenstackVolumeAuthenticationKey authenticationKey); Line 41: Line 42: @DELETE Line 43: @Path("{id}") Line 44: public Response remove(@PathParam("id") String id); > This "remove" method needs to be in the resource interface. This is a new r Done Line 45: Line 46: @Path("{id}") Line 47: OpenStackVolumeAuthenticationKeyResource getOpenStackVolumeAuthenticationKey(@PathParam("id") String id); https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml File backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml: Line 3917: openstack_volume_authentication_key.value: xs:string Line 3918: openstack_volume_authentication_key.usage_type: xs:string Line 3919: optionalArguments: Line 3920: openstack_volume_authentication_key.description: xs:string Line 3921: description: add a new authentication key to the OpenStack volume provider > No need to repeat the description when there is only one signature. Done Line 3922: - name: /openstackvolumeproviders/{openstackvolumeprovider:id}/authenticationkeys/{authenticationkey:id}|rel=update Line 3923: description: update the specified authentication key Line 3924: request: Line 3925: body: Line 3924: request: Line 3925: body: Line 3926: parameterType: OpenstackVolumeAuthenticationKey Line 3927: signatures: Line 3928: - mandatoryArguments: {} > Don't add this empty list: When not using an empty list, I get an error in "Generating RSDL files..." phase (InvocationTargetException: NullPointerException) Line 3929: optionalArguments: Line 3930: openstack_volume_authentication_key.value: xs:string Line 3931: openstack_volume_authentication_key.usage_type: xs:string Line 3932: openstack_volume_authentication_key.description: xs:string Line 3929: optionalArguments: Line 3930: openstack_volume_authentication_key.value: xs:string Line 3931: openstack_volume_authentication_key.usage_type: xs:string Line 3932: openstack_volume_authentication_key.description: xs:string Line 3933: description: update the specified authentication key > Same, no need to repeat the description. Done Line 3934: - name: /openstackvolumeproviders/{openstackvolumeprovider:id}/authenticationkeys|rel=delete Line 3935: description: delete the specified authentication key Line 3936: Line 3937: - name: /openstacknetworkproviders|rel=get https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackVolumeAuthenticationKeyResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackVolumeAuthenticationKeyResource.java: Line 35: Line 36: protected BackendOpenStackVolumeAuthenticationKeyResource(String providerId, String id) { Line 37: super(id, OpenstackVolumeAuthenticationKey.class, LibvirtSecret.class); Line 38: this.providerId = providerId; Line 39: this.guid = asGuidOr404(id); > I think that this assignment to "guid" already happens in the base class? D right, missed it.. done. Line 40: } Line 41: Line 42: @Override Line 43: public OpenstackVolumeAuthenticationKey get() { https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackVolumeAuthenticationKeysResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackVolumeAuthenticationKeysResource.java: Line 93: OpenStackVolumeAuthenticationKeyResource authenticationKeyResource = getOpenStackVolumeAuthenticationKey(id); Line 94: LibvirtSecret libvirtSecret = map(authenticationKeyResource.get(), null); Line 95: LibvirtSecretParameters parameters = new LibvirtSecretParameters(libvirtSecret); Line 96: return performAction(VdcActionType.RemoveLibvirtSecret, parameters); Line 97: } > The "remove" method needs to be moved to the entity resource because of Res Done Line 98: Line 99: @Override Line 100: @SingleEntityResource Line 101: public OpenStackVolumeAuthenticationKeyResource getOpenStackVolumeAuthenticationKey(String id) { https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackVolumeProvidersResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackVolumeProvidersResource.java: Line 47 Line 48 Line 49 Line 50 Line 51 > Are you removing the "certificates" sub-collection on purpose? yeah, volume providers don't support certificates.. it's relevant just for network providers. -- To view, visit https://gerrit.ovirt.org/41969 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1041a9760c29919d86da5479ce29db666d18a924 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
