Martin Mucha has posted comments on this change. Change subject: core, restapi: reporting & removal of unmanaged networks. ......................................................................
Patch Set 4: (8 comments) http://gerrit.ovirt.org/#/c/37525/4/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd File backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd: Line 3805: <xs:complexType name="UnmanagedNetwork"> Line 3806: <xs:complexContent> Line 3807: <xs:extension base="BaseResource"> Line 3808: <xs:sequence> Line 3809: <xs:element name="unmanaged_network_name" type="xs:string" minOccurs="1" maxOccurs="1"/> > The base "BaseResource" already has a "name" attribute, can we use it inste Done Line 3810: <xs:element name="nic_name" type="xs:string" minOccurs="0" maxOccurs="1"/> Line 3811: <xs:element name="nic_id" type="xs:string" minOccurs="0" maxOccurs="1"/> Line 3812: </xs:sequence> Line 3813: </xs:extension> Line 3805: <xs:complexType name="UnmanagedNetwork"> Line 3806: <xs:complexContent> Line 3807: <xs:extension base="BaseResource"> Line 3808: <xs:sequence> Line 3809: <xs:element name="unmanaged_network_name" type="xs:string" minOccurs="1" maxOccurs="1"/> > +1, in addition, the name of the network will be used as the identifier of needinfo: really hex2string? it's easier to do for me, but harder for user; If we're referencing network by name (which is really simple now — no spaces & special symbols, and I do believe that "some point" is quite distant), then it would be just right to use that name directly with "URI encoding" of problematic chars. That would solve "problematic chars" in future use and allow reference network using it's name. Line 3810: <xs:element name="nic_name" type="xs:string" minOccurs="0" maxOccurs="1"/> Line 3811: <xs:element name="nic_id" type="xs:string" minOccurs="0" maxOccurs="1"/> Line 3812: </xs:sequence> Line 3813: </xs:extension> Line 3807: <xs:extension base="BaseResource"> Line 3808: <xs:sequence> Line 3809: <xs:element name="unmanaged_network_name" type="xs:string" minOccurs="1" maxOccurs="1"/> Line 3810: <xs:element name="nic_name" type="xs:string" minOccurs="0" maxOccurs="1"/> Line 3811: <xs:element name="nic_id" type="xs:string" minOccurs="0" maxOccurs="1"/> > +1, also have the same comment on the discussion here: I've read that, but I did understand it that you suggest to *name* it host_nic, which I tried to disobey, due to existence equally named complex type. I've added that and also host element, which is required through reflection entaglement otherwise there won't be hrefs generated for unmanaged networks. Done. Line 3812: </xs:sequence> Line 3813: </xs:extension> Line 3814: Line 3815: </xs:complexContent> Line 3813: </xs:extension> Line 3814: Line 3815: </xs:complexContent> Line 3816: Line 3817: </xs:complexType> > Fix then indentation: two spaces per level. Done Line 3818: Line 3819: <xs:complexType name="HostNics"> Line 3820: <xs:complexContent> Line 3821: <xs:extension base="BaseResources"> http://gerrit.ovirt.org/#/c/37525/4/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendHostNetworkAttachmentsResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendHostNetworkAttachmentsResource.java: Line 11: import org.ovirt.engine.core.compat.Guid; Line 12: Line 13: public class BackendHostNetworkAttachmentsResource extends AbstractBackendNetworkAttachmentsResource { Line 14: Line 15: public BackendHostNetworkAttachmentsResource(Guid hostId) { > Remove the extra space. Done Line 16: super(hostId); Line 17: } Line 18: Line 19: @SingleEntityResource http://gerrit.ovirt.org/#/c/37525/4/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendUnmanagedNetworkResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendUnmanagedNetworkResource.java: Line 29: UnmanagedNetworkBusinessEntity entity = getEntity(UnmanagedNetworkBusinessEntity.class, Line 30: VdcQueryType.GetUnmanagedNetworkByVdsIdAndName, Line 31: new UnmanagedNetworkParameters(hostId, id), Line 32: id); Line 33: return map(entity, null); > If possible this method should just be: thanks for tip. Actually it's just shortcut for former code (which was missing adding links). So even former syntax did chech for 404, but this is nicer code. Done. Line 34: } Line 35: Line 36: @Override Line 37: protected Guid asGuidOr404(String id) { http://gerrit.ovirt.org/#/c/37525/4/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/UnmanagedNetworkMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/UnmanagedNetworkMapper.java: Line 13: Line 14: UnmanagedNetworkBusinessEntity entity = template == null ? new UnmanagedNetworkBusinessEntity() : template; Line 15: Line 16: if (model.isSetId()) { Line 17: entity.setId(model.getUnmanagedNetworkName()); > The backend identifier can't be directly copied to the RESTAPI identifier, needinfo: I asked moti in another comment: really string2hex? Since that make otherwise human-readable network name incomprehensible. Encoding that name like in URL will fix problematic letters and name will remain (mostly) readable. response when listing would then look like: <?xml version="1.0" encoding="UTF-8" standalone="yes"?> <unmanaged_networks> <unmanaged_network href="/api/hosts/a4dc841b-0958-42a3-93b8-e0109ef1f891/unmanagednetworks/big+john" id="big+john"> <name>big john</name> <nic_name>bond4</nic_name> <host_nic href="/api/hosts/a4dc841b-0958-42a3-93b8-e0109ef1f891/nics/c5a2aa63-b439-4b8e-916d-f28c48fa2705" id="c5a2aa63-b439-4b8e-916d-f28c48fa2705"/> <host href="/api/hosts/a4dc841b-0958-42a3-93b8-e0109ef1f891" id="a4dc841b-0958-42a3-93b8-e0109ef1f891"/> </unmanaged_network> </unmanaged_networks> Line 18: } Line 19: Line 20: if (model.isSetUnmanagedNetworkName()) { Line 21: entity.setNetworkName(model.getUnmanagedNetworkName()); Line 39: } Line 40: Line 41: UnmanagedNetwork model = template == null ? new UnmanagedNetwork() : template; Line 42: Line 43: model.setId(entity.getId()); > Same issue with identifiers. Need to encode/decode them. Done Line 44: model.setNicName(entity.getNicName()); Line 45: model.setNicId(entity.getNicId().toString()); Line 46: model.setUnmanagedNetworkName(entity.getNetworkName()); Line 47: -- To view, visit http://gerrit.ovirt.org/37525 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idcf6e1292ce6f3a335051123be8c3389e8230bf0 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Martin Mucha <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
