John,
Some comments on the recent commit:
http://git.fedorahosted.org/git/?p=spacewalk.git;a=commitdiff;h=d30dfdb992c0a3f3da09dabc3f604947bb3efe18
How often are we running 'deleteRecordsNotInList' in IndexManager? Is
this during every index run?
or in between runs?
For 'handleDeletedRecords' I'd move things around a bit. Instead of
wrapping these with the try/catch/finally:
+ query = databaseManager.getQuery(getQueryAllIds());
+ ids = query.loadList(Collections.EMPTY_MAP);
I'd wrap the entire thing in the try catch. Like this
protected int handleDeletedRecords(DatabaseManager databaseManager,
IndexManager indexManager)
throws SQLException {
HashSet<String> idSet = new HashSet();
Query<Long> query = null;
try {
List<Long> ids = query.loadList(Collections.EMPTY_MAP);
query = databaseManager.getQuery(getQueryAllIds());
if ((ids == null) || (ids.size() == 0)) {
log.warn("Got back no data from '" + getQueryAllIds() + "'");
log.warn("Skipping the handleDeletedRecords() method");
return 0;
}
for (Long num : ids) {
idSet.add(num.toString());
}
}
catch (SqlMapException e) {
e.printStackTrace();
log.warn("Error with 'getQueryAllIds()' on " +
super.getClass().toString());
//just print the warning so we know and skip this method.
return 0;
}
finally {
if (query != null) {
query.close();
}
}
return indexManager.deleteRecordsNotInList(idSet,
getIndexName(), getUniqueFieldId());
}
I like doing as much of the code within the try/catch blocks as I can.
IMO, it's easier to read the
normal flow. Also, make sure to check for null in a finally block.
Don't assume query was assigned.
No need to create a new String. Just return the string literal, Java
will do the right thing.
Change all of these from return new String("queryAllServerIds"); to
return "queryAllServerIds".
Also, I noticed quite a few log.warn statements. Are these really
warnings? Or are they more info?
What will the user do with that information? Will they need to fix
something? if not, I'd make them
info level.
jesus
_______________________________________________
Spacewalk-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/spacewalk-devel
