Alon Bar-Lev has posted comments on this change.

Change subject: oVirt Node Upgrade: Support N configuration
......................................................................


Patch Set 19:

(4 comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetoVirtISOsQuery.java
Line 48:             log.infoFormat("trying to match: nodeOS [{0}] to osPattern 
[{1}]",
Line 49:                 nodeOS.toLowerCase(),
Line 50:                 info.osPattern
Line 51:             );
Line 52:             if (!info.osPattern.matcher(nodeOS.toLowerCase()).find()) {
but we discussed using[1]:

     Pattern p = Pattern.compile("a*b");
     Matcher m = p.matcher("aaaaab");
     boolean b = m.matches();

[1] http://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html

Again, use Pattern.CASE_INSENSITIVE
Line 53:                 log.infoFormat(
Line 54:                     "Cannot match nodeOS [{0}] with pattern [{1}]",
Line 55:                      nodeOS.toLowerCase(),
Line 56:                      info.osPattern


Line 49:                 nodeOS.toLowerCase(),
Line 50:                 info.osPattern
Line 51:             );
Line 52:             if (!info.osPattern.matcher(nodeOS.toLowerCase()).find()) {
Line 53:                 log.infoFormat(
there is no ned for this debug message, it will just fill up logging with 
something that is not relevant, as you do have the log at beginning of loop.
Line 54:                     "Cannot match nodeOS [{0}] with pattern [{1}]",
Line 55:                      nodeOS.toLowerCase(),
Line 56:                      info.osPattern
Line 57:                 );


Line 57:                 );
Line 58:                 continue;
Line 59:             }
Line 60: 
Line 61:             if (info.path.isDirectory()) {
again, this is reached even if no match for ospattern in above loop.
Line 62:                 log.debugFormat("Looking for list of ISOs in [{0}]", 
info.path);
Line 63:                 log.debugFormat("regex isoPattern for ISOs: [{0}]", 
info.isoPattern);
Line 64:                 for (File file : info.path.listFiles()) {
Line 65:                     Matcher matcher = 
info.isoPattern.matcher(file.getName());


Line 155:                     isoFileName,
Line 156:                     pattern,
Line 157:                     rpmLike);
Line 158:         } else {
Line 159:             rpmLike = matcher.group(1).replaceAll("-", ".");
rpmversion does get '-', why do you replace it with dot?
Line 160:             isoVersion = new RpmVersion(rpmLike, "", true);
Line 161:             isoVersion.setRpmName(isoFileName);
Line 162:         }
Line 163:         return isoVersion;


-- 
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: 19
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsl...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsl...@redhat.com>
Gerrit-Reviewer: Michael Burns <mbu...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@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

Reply via email to