Martin Mucha has posted comments on this change. Change subject: core: refactored NetworkAttachmentCompleter + tests. ......................................................................
Patch Set 41: (2 comments) https://gerrit.ovirt.org/#/c/36142/41/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/NicNameNicIdCompleter.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/NicNameNicIdCompleter.java: Line 86: private void assertNicNameAndNicIdCoherence(Guid nicId, String nicName) { Line 87: VdsNetworkInterface interfaceById = existingNicsMap.get(nicId); Line 88: VdsNetworkInterface interfaceByName = existingNicsMap.get(nicName); Line 89: if (!Objects.equals(interfaceById, interfaceByName)) { Line 90: throw new IllegalArgumentException(NicNameNicIdCompleter.INCONSISTENT_NIC_IDENTIFICATION_ERROR_MESSAGE); > Noticed it while reviewing another patch- you should have candoaction for t I don't agree, since when used from rest it generates HTTP 400, which is correct. When used from UI, this is hardly user error, so nice message is not needed at all, since user cannot do anything with it. But I 'fixed' it. Done. Line 91: } Line 92: } Line 93: Line 94: interface NicNameAndNicIdAccessors { Line 86: private void assertNicNameAndNicIdCoherence(Guid nicId, String nicName) { Line 87: VdsNetworkInterface interfaceById = existingNicsMap.get(nicId); Line 88: VdsNetworkInterface interfaceByName = existingNicsMap.get(nicName); Line 89: if (!Objects.equals(interfaceById, interfaceByName)) { Line 90: throw new IllegalArgumentException(NicNameNicIdCompleter.INCONSISTENT_NIC_IDENTIFICATION_ERROR_MESSAGE); > I suggest here just ignoring it, and in the validator make sure the name an ignored here, added validation to HostSetupNetworkValidator, both for bond and network attachment, with it's id, and violating nicId and nicName. Done. Line 91: } Line 92: } Line 93: Line 94: interface NicNameAndNicIdAccessors { -- To view, visit https://gerrit.ovirt.org/36142 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I933333afdcb11bfcf9f761572229f7a7a7814381 Gerrit-PatchSet: 41 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@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
