Eli Mesika has posted comments on this change. Change subject: core: [RFE]Backup Awareness - DAO ......................................................................
Patch Set 1: (8 comments) https://gerrit.ovirt.org/#/c/39525/1/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java: Line 1274: > please use formatter. will fix https://gerrit.ovirt.org/#/c/39525/1/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/EngineBackupHistoryDAO.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/EngineBackupHistoryDAO.java: Line 1: package org.ovirt.engine.core.dao; Line 2: Line 3: import org.ovirt.engine.core.common.businessentities.EngineBackupHistory; Line 4: Line 5: public interface EngineBackupHistoryDAO extends DAO { > s/EngineBackupHistoryDAO/EngineBackupLogDao (also maintain camelCase) will fix Line 6: Line 7: /** Line 8: * Gets the last successful engine backup record Line 9: */ https://gerrit.ovirt.org/#/c/39525/1/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/EngineBackupHistoryDAODbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/EngineBackupHistoryDAODbFacadeImpl.java: Line 9: import java.sql.ResultSet; Line 10: import java.sql.SQLException; Line 11: Line 12: Line 13: public class EngineBackupHistoryDAODbFacadeImpl extends BaseDAODbFacade implements EngineBackupHistoryDAO { > please annotate the class with: will fix Line 14: Line 15: private static class EngineBackupHistoryRowMapper implements RowMapper<EngineBackupHistory> { Line 16: public static final EngineBackupHistoryRowMapper instance = new EngineBackupHistoryRowMapper(); Line 17: Line 16: instance > s/instance/INSTANCE ? (since it is static final...) will fix Line 25: return entity; Line 26: } Line 27: } Line 28: Line 29: private class EngineBackupHistorySqlParameterSource extends CustomMapSqlParameterSource { > this class isn't required. instead of using instances of it, please replace will fix Line 30: Line 31: public EngineBackupHistorySqlParameterSource(String dbName) { Line 32: super(getDialect()); Line 33: addValue("db_name", dbName); https://gerrit.ovirt.org/#/c/39525/1/backend/manager/modules/dal/src/main/jdbc-resources/engine-daos.properties File backend/manager/modules/dal/src/main/jdbc-resources/engine-daos.properties: Line 97: FenceAgentDAO=org.ovirt.engine.core.dao.FenceAgentDaoDbFacadeImpl Line 98: HostDeviceDao=org.ovirt.engine.core.dao.HostDeviceDaoDbFacadeImpl Line 99: StorageDeviceDao=org.ovirt.engine.core.dao.gluster.StorageDeviceDaoDbFacadeImpl Line 100: HostNicVfsConfigDao=org.ovirt.engine.core.dao.network.HostNicVfsConfigDaoDbFacadeImpl Line 101: EngineBackupHistoryDAO=org.ovirt.engine.core.dao.EngineBackupHistoryDAODbFacadeImpl > because this file was removed and required any more. will remove this file from the patch set https://gerrit.ovirt.org/#/c/39525/1/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/AuditLogDAOTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/AuditLogDAOTest.java: > the new stored procedure isn't covered by a test: OK , make sense , will add ... Line 1: package org.ovirt.engine.core.dao; Line 2: Line 3: import static org.junit.Assert.assertEquals; Line 4: import static org.junit.Assert.assertNotNull; https://gerrit.ovirt.org/#/c/39525/1/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/EngineBackupHistoryDAOTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/EngineBackupHistoryDAOTest.java: > https://gerrit.ovirt.org/#/c/39523/1/packaging/dbscripts/engine_backup_hist will add tests for that even if those are not called from engine Line 1: package org.ovirt.engine.core.dao; Line 2: Line 3: import org.junit.Before; Line 4: import org.junit.Test; -- To view, visit https://gerrit.ovirt.org/39525 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9b016e050f732eecb7f2872991d9fa9d31d9c024 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: [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
