Liron Ar has posted comments on this change.

Change subject: core: persist updated vm/template ovf config
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.ovirt.org/#/c/24178/4/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmAndTemplatesGenerationsDbFacadeImpl.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmAndTemplatesGenerationsDbFacadeImpl.java:

Line 15:         getCallsHandler().executeModification("UpdateOvfGenerations", 
getCustomMapSqlParameterSource()
Line 16:                 .addValue("vms_ids", StringUtils.join(ids, ','))
Line 17:                 .addValue("vms_db_generations", 
StringUtils.join(values, ','))
Line 18:                 .addValue("ovf_data", StringUtils.join(ovfData, 
"ENDOVF"))
Line 19:                 .addValue("ovf_data_seperator", "ENDOVF"));
> Move ENDOVF to be a constant.
Done
Line 20:     }
Line 21: 
Line 22:     @Override
Line 23:     public Long getOvfGeneration(Guid id) {


Line 60:                     @Override
Line 61:                     public Pair<Guid, String> mapRow(ResultSet 
resultSet, int i) throws SQLException {
Line 62:                         return new Pair<>(getGuid(resultSet, 
"vm_guid"), resultSet.getString("ovf_data"));
Line 63:                     }
Line 64:                 },
> extract this to a constant.
code leftover that i forgot to extract from here, done..tnx.
Line 65:                 getCustomMapSqlParameterSource().addValue("ids", 
StringUtils.join(ids, ',')));
Line 66:     }


http://gerrit.ovirt.org/#/c/24178/4/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmAndTemplatesGenerationsDaoTest.java
File 
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmAndTemplatesGenerationsDaoTest.java:

Line 73:     }
Line 74: 
Line 75:     @Test
Line 76:     public void testGetVmTemplatesIdsForOvfUpdateOneTemplate() {
Line 77:         
vmAndTemplatesGenerationsDAO.updateOvfGenerations(Collections.singletonList(FixturesTool.VM_TEMPLATE_RHEL5),
 Collections.singletonList(Long.valueOf(0)), Arrays.asList("a"));
> Collections.singletonList
don't need immutabillity here, so there's no preference to use it.
Line 78: 
Line 79:         List<Guid> guids = 
vmAndTemplatesGenerationsDAO.getVmTemplatesIdsForOvfUpdate(FixturesTool.STORAGE_POOL_RHEL6_ISCSI_OTHER);
Line 80:         assertEquals("one template should need ovf update", 1, 
guids.size());
Line 81:         assertEquals("wrong template returned as in need for ovf 
update", guids.get(0), FixturesTool.VM_TEMPLATE_RHEL5);


Line 146:     }
Line 147: 
Line 148:     @Test
Line 149:     public void testGetVmssIdsForOvfUpdateOneVm() {
Line 150:         
vmAndTemplatesGenerationsDAO.updateOvfGenerations(Collections.singletonList(FixturesTool.VM_RHEL5_POOL_50),
 Collections.singletonList(Long.valueOf(0)), Arrays.asList("a"));
> Collections.singletonList
same
Line 151: 
Line 152:         List<Guid> guids = 
vmAndTemplatesGenerationsDAO.getVmsIdsForOvfUpdate(FixturesTool.STORAGE_POOL_RHEL6_ISCSI_OTHER);
Line 153:         assertEquals("one vm should need ovf update", 1, 
guids.size());
Line 154:         assertEquals("wrong vm returned as in need for ovf update", 
guids.get(0), FixturesTool.VM_RHEL5_POOL_50);


http://gerrit.ovirt.org/#/c/24178/4/packaging/dbscripts/vms_sp.sql
File packaging/dbscripts/vms_sp.sql:

Line 39:    AS $procedure$
Line 40: BEGIN
Line 41: RETURN QUERY SELECT *
Line 42:    FROM vm_ovf_generations ovf
Line 43:    WHERE ovf.vm_guid IN (SELECT * FROM fnSplitterUuid(v_ids));
> is the SELECT * really required?
yes, because i want to have the returned id and the ovf to map between them 
(e.g ovf for id YYY is AAA)
Line 44: END; $procedure$
Line 45: LANGUAGE plpgsql;
Line 46: 
Line 47: 


-- 
To view, visit http://gerrit.ovirt.org/24178
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic891429a2a7d055fbd1927e878ae7e0f8b7c9fd9
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to