Juan Hernandez has posted comments on this change. Change subject: restapi: Phase 5: Add graphics console representation ......................................................................
Patch Set 9: Code-Review+1 (3 comments) https://gerrit.ovirt.org/#/c/39057/9/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmGraphicsConsoleResource.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmGraphicsConsoleResource.java: Line 4: import javax.ws.rs.Produces; Line 5: import org.ovirt.engine.api.model.GraphicsConsole; Line 6: import javax.ws.rs.core.Response; Line 7: Line 8: public interface VmGraphicsConsoleResource { As this (having two methods to handle different media types) is somewhat unusual I'd appreciate if you can add javadoc comments explaining it. Line 9: @GET Line 10: @Produces({ ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, Line 11: ApiMediaType.APPLICATION_X_YAML}) Line 12: public GraphicsConsole get(); https://gerrit.ovirt.org/#/c/39057/9/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmGraphicsConsolesResource.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmGraphicsConsolesResource.java: Line 12: @GET Line 13: public GraphicsConsoles list(); Line 14: Line 15: @Path("{iden}") Line 16: public VmGraphicsConsoleResource getVmConsoleResource(@PathParam("iden") String id); Consider renaming this method to "getVmGraphicsConsoleResource". Not completely sure but I think we use reflection to calculate links, and we expect a match between the returned type and the name of the method. Line 17: https://gerrit.ovirt.org/#/c/39057/9/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmGraphicsConsoleResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmGraphicsConsoleResource.java: Line 64: new ConfigureConsoleOptionsParams(consoleOptions, true)).getReturnValue(); Line 65: Line 66: String descriptor = runQuery(VdcQueryType.GetConsoleDescriptorFile, new ConsoleOptionsParams(configuredOptions)) Line 67: .getReturnValue(); Line 68: Response.ResponseBuilder builder = Response.ok(descriptor.getBytes(), ApiMediaType.APPLICATION_X_VIRT_VIEWER); Findbugs will complain that the character encoding is not explicitly specified: descriptor.getBytes(Charset.forName("UTF-8")) Line 69: Line 70: return builder.build(); Line 71: } -- To view, visit https://gerrit.ovirt.org/39057 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7dbc63ac49bfba5637c38b84b373d78f348c7872 Gerrit-PatchSet: 9 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@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
