Allon Mureinik has posted comments on this change.
Change subject: core: Introducing batch to async task mgr
......................................................................
Patch Set 7: I would prefer that you didn't submit this
(7 inline comments)
See inline.
My main concern is about the BE definition, the others are style issues.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
Line 1367
Line 1368
Line 1369
Line 1370
Line 1371
Please also remove the EMPTY_GUID_ARRAY constant - it is not used anywhere else.
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/AsyncTaskEntity.java
Line 3: import org.ovirt.engine.core.common.VdcObjectType;
Line 4: import org.ovirt.engine.core.common.utils.ObjectUtils;
Line 5: import org.ovirt.engine.core.compat.Guid;
Line 6:
Line 7: public class AsyncTaskEntity implements BusinessEntity<Guid> {
I don't think this should be a BusinessEntity - see explanation below.
Where is it needed anyway?
Line 8:
Line 9: private static final long serialVersionUID = -5368929756190062327L;
Line 10: private Guid taskId;
Line 11: private VdcObjectType entityType;
Line 32: return true;
Line 33: if (obj == null)
Line 34: return false;
Line 35: if (getClass() != obj.getClass())
Line 36: return false;
please add brackets to all of these ifs
Line 37:
Line 38: AsyncTaskEntity other = (AsyncTaskEntity) obj;
Line 39: return ObjectUtils.objectsEqual(taskId, other.taskId) &&
Line 40: ObjectUtils.objectsEqual(entityId, other.entityId) &&
Line 44: public AsyncTaskEntity(Guid taskId, VdcObjectType entityType, Guid
entityId) {
Line 45: this.taskId = taskId;
Line 46: this.entityType = entityType;
Line 47: this.entityId = entityId;
Line 48: }
Please move this up to group all the ctors together
Line 49:
Line 50: public Guid getEntityId() {
Line 51: return entityId;
Line 52: }
Line 72: }
Line 73:
Line 74: @Override
Line 75: public Guid getId() {
Line 76: return taskId;
BusinessEntities silently assume that IDs are unique.
Here, you can have several AsyncTaskEntities with the same ID (e.g., a task
that is merging two snapshots)
Line 77: }
Line 78:
Line 79: @Override
Line 80: public void setId(Guid id) {
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAODbFacadeImpl.java
Line 51: public AsyncTaskEntity mapRow(ResultSet rs, int rowNum) throws
SQLException {
Line 52: AsyncTaskEntity entity = new AsyncTaskEntity();
Line 53: entity.setEntityId(getGuid(rs, "entity_id"));
Line 54: entity.setTaskId(getGuid(rs, "async_task_id"));
Line 55:
entity.setEntityType(VdcObjectType.valueOf(rs.getString("entity_type")));
why is this a string and not an int?
Line 56: return entity;
Line 57: }
Line 58: }
Line 59:
Line 119: public MapSqlParameterSource map(AsyncTaskEntity entity) {
Line 120: CustomMapSqlParameterSource paramSource =
getCustomMapSqlParameterSource();
Line 121: paramSource.addValue("task_id", entity.getTaskId()).
Line 122: addValue("entity_id", entity.getEntityId()).
Line 123: addValue("entity_type",
entity.getEntityType().toString());
same question
Line 124: return paramSource;
Line 125:
Line 126: }
Line 127: };
--
To view, visit http://gerrit.ovirt.org/16445
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6d8dc6095430c99fdb6c8cd8289c23e270fff9e
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
