Mike Kolesnik has posted comments on this change.
Change subject: engine: Extract vnic profile helper methods to VnicProfileHelper
......................................................................
Patch Set 1:
(6 comments)
....................................................
Commit Message
Line 7: engine: Extract vnic profile helper methods to VnicProfileHelper
Line 8:
Line 9: The patch extracts the methods used to resolve the vnic profile
Line 10: for a given vnic into a new helper so it can be properly reused.
Line 11: Signed-off-by: Moti Asayag <masa...@redhat.com>
Not sure that you need to sign off twice..
Line 12:
Line 13: Change-Id: Ic03ae924a6fb31a948723a57b331597ec7d76977
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/VnicProfileHelper.java
Line 34: private Map<String, Network> networksInClusterByName;
Line 35: private List<VnicProfileView> vnicProfilesInDc;
Line 36: private Version compatibilityVersion;
Line 37:
Line 38: public VnicProfileHelper() {
Why empty c'tor?
Line 39: }
Line 40:
Line 41: public VnicProfileHelper(Guid clusterId, Guid dataCenterId,
Version compatibilityVersion) {
Line 42: invalidNetworkNames = new ArrayList<>();
Line 38: public VnicProfileHelper() {
Line 39: }
Line 40:
Line 41: public VnicProfileHelper(Guid clusterId, Guid dataCenterId,
Version compatibilityVersion) {
Line 42: invalidNetworkNames = new ArrayList<>();
Not sure why initialize this here and not on declaration..
Line 43: invalidIfaceNames = new ArrayList<>();
Line 44: networksInClusterByName =
Entities.entitiesByName(getNetworkDao().getAllForCluster(clusterId));
Line 45: vnicProfilesInDc =
getVnicProfileViewDao().getAllForDataCenter(dataCenterId);
Line 46: this.compatibilityVersion = compatibilityVersion;
Line 40:
Line 41: public VnicProfileHelper(Guid clusterId, Guid dataCenterId,
Version compatibilityVersion) {
Line 42: invalidNetworkNames = new ArrayList<>();
Line 43: invalidIfaceNames = new ArrayList<>();
Line 44: networksInClusterByName =
Entities.entitiesByName(getNetworkDao().getAllForCluster(clusterId));
Generally it's not a good idea to perform DB look-ups in the c'tor, please
consider the lazy init approach.
Line 45: vnicProfilesInDc =
getVnicProfileViewDao().getAllForDataCenter(dataCenterId);
Line 46: this.compatibilityVersion = compatibilityVersion;
Line 47: }
Line 48:
Line 45: vnicProfilesInDc =
getVnicProfileViewDao().getAllForDataCenter(dataCenterId);
Line 46: this.compatibilityVersion = compatibilityVersion;
Line 47: }
Line 48:
Line 49: public void updateNicWithVnicProfileForUser(VmNetworkInterface
vmInterface, Guid userId) {
How about a javadoc of what this method does?
Line 50: if (!updateNicWithVnicProfile(vmInterface, userId)) {
Line 51: markNicHasNoProfile(vmInterface);
Line 52: }
Line 53: }
Line 132: invalidIfaceNames.add(iface.getName());
Line 133: iface.setVnicProfileId(null);
Line 134: }
Line 135:
Line 136: public void auditInvalidInterfaces(String entityName,
AuditLogType logType) {
Why have AuditLogType as parameter and not receive it in the c'tor?
Line 137: if (!invalidNetworkNames.isEmpty()) {
Line 138: AuditLogableBase logable = new AuditLogableBase();
Line 139: logable.addCustomValue("EntityName", entityName);
Line 140: logable.addCustomValue("Networks",
StringUtils.join(invalidNetworkNames, ','));
--
To view, visit http://gerrit.ovirt.org/18797
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic03ae924a6fb31a948723a57b331597ec7d76977
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com>
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
