Allon Mureinik has posted comments on this change.
Change subject: core: introducing OvfAutoUpdate
......................................................................
Patch Set 21: Fails
(7 inline comments)
-1 given for missing validation code that was mistakenly pushed in the next
patch in the series, just so this doesn't get merged by mistake.
Also, not some inline issues.
....................................................
File backend/manager/dbscripts/storages_sp.sql
Line 99: FROM vms
Line 100: WHERE storage_pool_id = v_id);
Line 101: delete FROM vm_static where vm_guid in (select vm_guid from vms
where storage_pool_id = v_id);
Line 102:
Line 103: -- Get (and keep) a shared lock with "right to upgrade to
exclusive"
replacing the tab with spaces in this procedure is unrelated to the patch.
If you're going to do it please separate it to a different patch, and also
apply the same fix to the next line.
Line 104: -- in order to force locking parent before children
Line 105: select id INTO v_val FROM storage_pool WHERE id = v_id FOR
UPDATE;
Line 106:
Line 107: DELETE FROM storage_pool
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfDataUpdater.java
Line 42: import org.ovirt.engine.core.utils.timer.OnTimerMethodAnnotation;
Line 43: import org.ovirt.engine.core.utils.timer.SchedulerUtil;
Line 44: import org.ovirt.engine.core.utils.timer.SchedulerUtilQuartzImpl;
Line 45:
Line 46: public class OvfDataUpdater {
General:
Throughout the class - either document the empty @param and @return statements,
or remove them.
[Sorry for not noticing earlier]
Line 47: private static final Log log =
LogFactory.getLog(OvfDataUpdater.class);
Line 48: private static final OvfDataUpdater INSTANCE = new
OvfDataUpdater();
Line 49: private int itemsCountPerUpdate;
Line 50: protected static final int MAX_ITEMS_PER_SQL_STATEMENT = 100;
Line 176: }
Line 177: }
Line 178:
Line 179: /**
Line 180: * create and returns map contains valid templates metadata
"Creates and returns a map containing valid templates metadata"
Line 181: * @param idsToProcess
Line 182: * @return
Line 183: */
Line 184: protected Map<Guid, KeyValuePairCompat<String, List<Guid>>>
populateTemplatesMetadataForOvfUpdate(List<Guid> idsToProcess) {
....................................................
File
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDAOTest.java
Line 108: * Asserts the given VM is the expected one
Line 109: */
Line 110: private void assertGetResult(VM result) {
Line 111: assertNotNull(result);
Line 112: assertTrue("Vm db generation wasn't loaded as expected",
result.getDbGeneration() == 1);
Wait, this is worse than the previous version.
What happened to the assertEquals()?
Line 113: assertEquals(result, existingVm);
Line 114: }
Line 115:
Line 116: /**
Line 351: assertNotNull(result);
Line 352: assertFalse(result.isEmpty());
Line 353: assertEquals(VM_COUNT, result.size());
Line 354: for (VM vm : result) {
Line 355: assertTrue("Vm db generation wasn't loaded as expected",
vm.getDbGeneration() == 1);
Wait, this is worse than the previous version.
What happened to the assertEquals()?
Line 356: }
Line 357: }
....................................................
File
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmTemplateDAOTest.java
Line 271:
Line 272: private static void assertGetResult(VmTemplate result) {
Line 273: assertNotNull(result);
Line 274: assertEquals(EXISTING_TEMPLATE_ID, result.getId());
Line 275: assertTrue("Template generation wasn't loaded as expected",
result.getDbGeneration() == 1);
Wait, this is worse than the previous version.
What happened to the assertEquals()?
[it supports primitives, despite what the name may suggest]
Line 276: }
Line 277:
Line 278: private static void assertGetAllResult(List<VmTemplate> result) {
Line 279: assertNotNull(result);
Line 278: private static void assertGetAllResult(List<VmTemplate> result) {
Line 279: assertNotNull(result);
Line 280: assertFalse(result.isEmpty());
Line 281: for (VmTemplate template : result){
Line 282: assertTrue("Template generation wasn't loaded as
expected", template.getDbGeneration() == 1);
Wait, this is worse than the previous version.
What happened to the assertEquals()?
Line 283: }
Line 284: }
--
To view, visit http://gerrit.ovirt.org/9328
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9b5132300fb1f1fd94f771cab15efe5246dbeca8
Gerrit-PatchSet: 21
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: liron aravot <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
