Michael Pasternak has posted comments on this change.
Change subject: restapi: REST apis for SWIFT maintenance
......................................................................
Patch Set 8: I would prefer that you didn't submit this
(17 inline comments)
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/GlusterServiceActionType.java
Line 2:
Line 3: public enum GlusterServiceActionType {
Line 4: start,
Line 5: stop,
Line 6: restart;
enum members should be in upper case
Line 7:
Line 8: public int getValue() {
Line 9: return this.ordinal();
Line 10: }
Line 5: stop,
Line 6: restart;
Line 7:
Line 8: public int getValue() {
Line 9: return this.ordinal();
should return name().toLowerCase();
Line 10: }
Line 11:
Line 12: public static GlusterServiceActionType valueOf(int value) {
Line 13: return values()[value];
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
Line 1345: <xs:element name="os" type="OperatingSystem" minOccurs="0"
maxOccurs="1"/>
Line 1346: <xs:element ref="hooks" minOccurs="0"/>
Line 1347: <xs:element name="libvirt_version" type="Version"
minOccurs="0" maxOccurs="1"/>
Line 1348: <!-- Optionally specify the display address of this host
explicitly -->
Line 1349: <xs:element ref="services" minOccurs="0"/>
not needed
Line 1350: <xs:element ref="display" minOccurs="0"/>
Line 1351: </xs:sequence>
Line 1352: </xs:extension>
Line 1353: </xs:complexContent>
Line 1753: <xs:complexContent>
Line 1754: <xs:extension base="BaseResource">
Line 1755: <xs:sequence>
Line 1756: <xs:element ref="host" minOccurs="0" />
Line 1757: <xs:element name="status" type="xs:string" minOccurs="0" />
please reuse <status> type
Line 1758: <xs:element name="service_type" type="xs:string"
minOccurs="0" />
Line 1759: </xs:sequence>
Line 1760: </xs:extension>
Line 1761: </xs:complexContent>
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/gluster/BackendServiceResource.java
Line 25: }
Line 26:
Line 27: @Override
Line 28: protected Service addParents(Service model) {
Line 29: model.setId(id);
at this stage model should already have an id
Line 30: parent.addParents(model);
Line 31: return model;
Line 32: }
Line 33:
Line 32: }
Line 33:
Line 34: @Override
Line 35: public Service get() {
Line 36: Services serviceRes = getParent().list();
no dedicated query for get()?
Line 37: if (serviceRes != null) {
Line 38: for (Service srvc : serviceRes.getServices()) {
Line 39: if (srvc.getId().equals(id)) {
Line 40: return srvc;
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/gluster/BackendServicesResource.java
Line 42: }
Line 43:
Line 44: public BackendServicesResource(BackendHostResource parent, String
serverId) {
Line 45: this(serverId);
Line 46: this.serverId = serverId;
no need for the this.serverId, host id defined at parent
Line 47: setParent(parent);
Line 48: }
Line 49:
Line 50: public BackendHostResource getParent() {
Line 88: return model;
Line 89: }
Line 90:
Line 91: private void validateParameters(Action action) {
Line 92: String srvcTypeParam = action.getServiceType();
any reason for this local var?
Line 93: validateParameters(action, "serviceType");
Line 94: validateEnum(ServiceType.class, srvcTypeParam);
Line 95: }
Line 96:
Line 104: @Override
Line 105: @POST
Line 106: @Consumes({ "application/xml", "application/json",
"application/x-yaml" })
Line 107: @Actionable
Line 108: @Path("start")
no need for annotations in this context, they're defined at the interface
Line 109: public Response start(Action action) {
Line 110: validateParameters(action);
Line 111: return performServiceAction(action,
GlusterServiceActionType.start);
Line 112: }
Line 106: @Consumes({ "application/xml", "application/json",
"application/x-yaml" })
Line 107: @Actionable
Line 108: @Path("start")
Line 109: public Response start(Action action) {
Line 110: validateParameters(action);
according to rsdl_metadata.yaml you should be validating these fields:
action.name: 'xs:string', action.service_type: 'xs:string'
Line 111: return performServiceAction(action,
GlusterServiceActionType.start);
Line 112: }
Line 113:
Line 114: @Override
Line 114: @Override
Line 115: @POST
Line 116: @Consumes({ "application/xml", "application/json",
"application/x-yaml" })
Line 117: @Actionable
Line 118: @Path("stop")
no need for annotations in this context, they're defined at the interface
Line 119: public Response stop(Action action) {
Line 120: validateParameters(action);
Line 121: return performServiceAction(action,
GlusterServiceActionType.stop);
Line 122: }
Line 116: @Consumes({ "application/xml", "application/json",
"application/x-yaml" })
Line 117: @Actionable
Line 118: @Path("stop")
Line 119: public Response stop(Action action) {
Line 120: validateParameters(action);
according to rsdl_metadata.yaml you should be validating these fields:
action.name: 'xs:string', action.service_type: 'xs:string'
Line 121: return performServiceAction(action,
GlusterServiceActionType.stop);
Line 122: }
Line 123:
Line 124: @Override
Line 124: @Override
Line 125: @POST
Line 126: @Consumes({ "application/xml", "application/json",
"application/x-yaml" })
Line 127: @Actionable
Line 128: @Path("restart")
no need for annotations in this context, they're defined at the interface
Line 129: public Response restart(Action action) {
Line 130: validateParameters(action);
Line 131: return performServiceAction(action,
GlusterServiceActionType.restart);
Line 132: }
Line 126: @Consumes({ "application/xml", "application/json",
"application/x-yaml" })
Line 127: @Actionable
Line 128: @Path("restart")
Line 129: public Response restart(Action action) {
Line 130: validateParameters(action);
according to rsdl_metadata.yaml you should be validating these fields:
action.name: 'xs:string', action.service_type: 'xs:string'
Line 131: return performServiceAction(action,
GlusterServiceActionType.restart);
Line 132: }
Line 133:
Line 134: @Override
....................................................
File
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/ServiceMapper.java
Line 23: service.setName(fromService.getServiceName());
Line 24: }
Line 25:
Line 26: if(fromService.getStatus() != null) {
Line 27: service.setStatus(fromService.getStatus().name());
should happen via public enum mapping
Line 28: }
Line 29:
Line 30: if(fromService.getServiceType() != null) {
Line 31:
service.setServiceType(fromService.getServiceType().name());
Line 27: service.setStatus(fromService.getStatus().name());
Line 28: }
Line 29:
Line 30: if(fromService.getServiceType() != null) {
Line 31:
service.setServiceType(fromService.getServiceType().name());
should happen via public enum mapping
Line 32: }
Line 33:
Line 34: return service;
Line 35: }
....................................................
Commit Message
Line 18: - api/hosts/{id}/services/stop
Line 19: stops all the services in the said host for a given type
Line 20: - api/hosts/{id}/services/restart
Line 21: re-starts all the services in the said host for a given type
Line 22:
please add:
1. new feature to the FeatureHelper
2. describe new ENUMs at the /capabilities resource
Line 23: Change-Id: I87cb354e10fed21cbd18b874595b726d3ac018a6
--
To view, visit http://gerrit.ovirt.org/16691
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I87cb354e10fed21cbd18b874595b726d3ac018a6
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
