Alon Bar-Lev has posted comments on this change.
Change subject: [WIP] oVirt Node Upgrade: Support N configurations
......................................................................
Patch Set 3: (7 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetoVirtISOsQuery.java
Line 176: }
Line 177: return null;
Line 178: }
Line 179:
Line 180: private List<String> getListOfIsoFiles(File directory,
OVirtNodeInfo info) {
why have you removed the static? not that it is that important... :)
Line 181: List<String> isoFileList = new ArrayList<String>();
Line 182: File[] filterOvirtFiles = filterOvirtFiles(directory,
getIsoPattern(info));
Line 183: for (File file : filterOvirtFiles) {
Line 184: isoFileList.add(file.getName());
Line 253: }
Line 254: });
Line 255: }
Line 256:
Line 257: private boolean isIsoVersionSupported(Version isoVersion,
OVirtNodeInfo Info) {
Why have you removed the static?
Line 258: boolean result = false;
Line 259:
Line 260: Version supported = new Version(Info.minimumVersion);
Line 261: if (isoVersion.compareTo(supported) > 0) {
Line 278: * Since the prefix from the configuration may change (reloadable
configuration), it is checked each time.
Line 279: * A cached version of pattern is saved, though, to avoid the
overhead of re-compiling it.
Line 280: */
Line 281: private Pattern getIsoPattern(OVirtNodeInfo info) {
Line 282: String expectedPattern = "^(" +
info.prefix.replace(info.delimiter, "|") + ")" + ".*.iso";
I thought we agreed to put regular expression within the database, so we can
customized this in future.
Why do you need the delimiter here?
Line 283: if (isoPattern == null ||
!expectedPattern.equals(isoPattern.toString())) {
Line 284: isoPattern = Pattern.compile(expectedPattern);
Line 285: }
Line 286: return isoPattern;
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallVdsCommand.java
Line 279: * @param isoFile
Line 280: * the ISO file for upgrade
Line 281: * @return true if ISO is compatible with oVirt node OS version
or if failed to resolve Host or RHEV-H version
Line 282: */
Line 283: public boolean isIsoVersionCompatible(RpmVersion ovirtOsVersion,
String isoFile, OVirtNodeInfo[] info) {
Can't we reuse some of the logic of previous file here too?
Line 284: boolean retValue = true;
Line 285: if (ovirtOsVersion != null) {
Line 286: try {
Line 287: for (OVirtNodeInfo infoData : info) {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OVirtNodeInfo.java
Line 1: package org.ovirt.engine.core.bll;
Line 2:
Line 3: import java.util.LinkedList;
Line 4: import java.util.List;
I think here there should be a line of space between standard and local
packages.
Line 5: import org.ovirt.engine.core.common.config.Config;
Line 6: import org.ovirt.engine.core.common.config.ConfigValues;
Line 7:
Line 8: public class OVirtNodeInfo {
Line 8: public class OVirtNodeInfo {
Line 9: public String prefix;
Line 10: public String path;
Line 11: public String minimumVersion;
Line 12: public String delimiter = ":";
I think this should be private and final.
Also the convention is probably to have it upercase and before non-constants.
Line 13:
Line 14: public OVirtNodeInfo(String prefix, String path, String
minimumVersion) {
Line 15: this.prefix = prefix;
Line 16: this.path = path;
Line 17: this.minimumVersion = minimumVersion;
Line 18: }
Line 19:
Line 20: public static OVirtNodeInfo[] get() {
Line 21: final String prefix[] = Config.<String>
GetValue(ConfigValues.OvirtIsoPrefix).split(":");
":" -> delimiter?
Line 22: final String path[] =
Config.resolveOVirtISOsRepositoryPath().split(":");
Line 23: final String minimumVersion[] = Config.<String>
GetValue(ConfigValues.OvirtInitialSupportedIsoVersion).split(":");
Line 24:
Line 25: if (prefix.length != minimumVersion.length ||
--
To view, visit http://gerrit.ovirt.org/14756
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb9dc5d0dc8780b519107acbe0ae866831f782c
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Michael Burns <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
