Moti Asayag has posted comments on this change. Change subject: core: Remove usage of dynamic queries from the Backend ......................................................................
Patch Set 8: (4 comments) http://gerrit.ovirt.org/#/c/24310/8/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendStorageDomainContentResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendStorageDomainContentResource.java: Line 84: protected VDSGroup lookupClusterByName(String name) { Line 85: return getEntity(VDSGroup.class, Line 86: VdcQueryType.GetVdsGroupByName, Line 87: new NameQueryParameters(name), Line 88: name); Juan, The last argument is being used also in the 404 message which makes its content clear. Especially since in some cases we fetch entity which is related to the current resource, and not the resource itself. Would you consider preserving it ? (should be applied across the patch) Line 89: } Line 90: Line 91: protected VDSGroup lookupClusterById(String id) { Line 92: return getEntity(VDSGroup.class, http://gerrit.ovirt.org/#/c/24310/8/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendHostsResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendHostsResource.java: Line 174: return collection; Line 175: } Line 176: Line 177: private Guid getClusterId(Host host) { Line 178: String name = (host.isSetCluster() && host.getCluster().isSetName() the surrounding brackets seems redundant. Line 179: ? host.getCluster().getName() Line 180: : "Default"); Line 181: return host.isSetCluster() && host.getCluster().isSetId() Line 182: ? new Guid(host.getCluster().getId()) http://gerrit.ovirt.org/#/c/24310/8/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendAssignedPermissionsResourceTest.java File backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendAssignedPermissionsResourceTest.java: Line 66: new String[] {}, Line 67: new Object[] {}, Line 68: getUsers()); Line 69: Line 70: // setUpGetEntityExpectations(VdcQueryType.Search, any reason to save the commented lines ? Line 71: // SearchParameters.class, Line 72: // new String[] {"SearchPattern", "SearchTypeValue"}, Line 73: // new Object[] {"users:", SearchType.DBUser}, Line 74: // getUsers()); http://gerrit.ovirt.org/#/c/24310/8/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendHostsResourceTest.java File backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendHostsResourceTest.java: Line 201: } Line 202: Line 203: @Test Line 204: public void testAddHostClusterByName() throws Exception { Line 205: setUriInfo(setUpBasicUriExpectations()); same here Line 206: // setUpGetEntityExpectations("Cluster: name=" + NAMES[1], Line 207: // SearchType.Cluster, Line 208: // setUpVDSGroup(GUIDS[1])); Line 209: -- To view, visit http://gerrit.ovirt.org/24310 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3df981a958fae36edd6ecf3cb2bb47b94b2c446a Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
