Mike Kolesnik has posted comments on this change.
Change subject: core: Don't create Guid from String in DAO module
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(4 inline comments)
Please note there are some places that you changed new
Guid(rs.getString("...")) to getGuid(rs, "...") which is a change in behaviour
from throwing NPE to not throwing NPE.
When refactoring you should avoid such changes that change external behaviour
of the code that you're refactoring.
These changes can be done in a separate patch, and would need to be checked
more thoroughly.
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDAODbFacadeImpl.java
Line 133: .getTimestamp("lastModified")));
Line 134: entity.setAppList(rs.getString("app_list"));
Line 135:
entity.setStorageIds(GuidUtils.getGuidListFromString(rs.getString("storage_id")));
Line 136:
entity.setStoragesNames(split(rs.getString("storage_name")));
Line 137: entity.setVmSnapshotId(getGuidDefaultEmpty(rs,
"vm_snapshot_id"));
Why default to empty here?
Line 138: entity.setVolumeType(VolumeType.forValue(rs
Line 139: .getInt("volume_type")));
Line 140: entity.setvolumeFormat(VolumeFormat.forValue(rs
Line 141: .getInt("volume_format")));
Line 140: entity.setvolumeFormat(VolumeFormat.forValue(rs
Line 141: .getInt("volume_format")));
Line 142: entity.setId(getGuidDefaultEmpty(rs, "image_group_id"));
Line 143:
entity.setStoragePath(split(rs.getString("storage_path")));
Line 144: entity.setStoragePoolId(getGuidDefaultEmpty(rs,
"storage_pool_id"));
Why default to empty here?
Line 145: entity.setBoot(rs.getBoolean("boot"));
Line 146: entity.setReadRate(rs.getInt("read_rate"));
Line 147: entity.setWriteRate(rs.getInt("write_rate"));
Line 148:
entity.setReadLatency(rs.getObject("read_latency_seconds") != null ?
rs.getDouble("read_latency_seconds")
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmPoolDAODbFacadeImpl.java
Line 192:
Line 193: @Override
Line 194: public VmPoolMap mapRow(ResultSet rs, int rowNum) throws
SQLException {
Line 195: VmPoolMap entity = new VmPoolMap();
Line 196: entity.setvm_guid(getGuid(rs, "vm_guid"));
Why not default empty?
Line 197: entity.setvm_pool_id(getGuid(rs, "vm_pool_id"));
Line 198: return entity;
Line 199: }
Line 200: }
Line 193: @Override
Line 194: public VmPoolMap mapRow(ResultSet rs, int rowNum) throws
SQLException {
Line 195: VmPoolMap entity = new VmPoolMap();
Line 196: entity.setvm_guid(getGuid(rs, "vm_guid"));
Line 197: entity.setvm_pool_id(getGuid(rs, "vm_pool_id"));
Same here..
Line 198: return entity;
Line 199: }
Line 200: }
Line 201:
--
To view, visit http://gerrit.ovirt.org/16392
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f6aec5aa512638c2dbfaa567784f295d01e5ac9
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
