OZ-532: Force release liquibase lock before EIP client startup #34
Conversation
mks-d
changed the title
| @@ -26,6 +26,7 @@ | |||
| <maven.compiler.source>17</maven.compiler.source> | |||
| <maven.compiler.target>17</maven.compiler.target> | |||
| <start-class>com.ozonehis.eip.Application</start-class> | |||
| <liquibaseVersion>4.24.0</liquibaseVersion> | |||
There was a problem hiding this comment.
Why not use the latest? v4.27.0
There was a problem hiding this comment.
Just to be consistent across services https://github.com/openmrs/openmrs-eip/blob/master/pom.xml#L33
There was a problem hiding this comment.
Good work @VaishSiddharth!
I have provided you with some suggestions
| @Value("${spring.mngt-datasource.jdbcUrl:#{null}}") | ||
| private String url; | ||
|
|
||
| @Value("${spring.mngt-datasource.username:#{null}}") | ||
| private String username; | ||
|
|
||
| @Value("${spring.mngt-datasource.password:#{null}}") | ||
| private String password; |
There was a problem hiding this comment.
There is no need to do this since we already have the management datasource class here
| if (url == null || username == null || password == null) { | ||
| log.info("Skipping liquibase step: forceReleaseLock"); | ||
| return; | ||
| } | ||
| Connection connection = null; | ||
| try { | ||
| connection = DriverManager.getConnection(url, username, password); | ||
| Database database = | ||
| DatabaseFactory.getInstance().findCorrectDatabaseImplementation(new JdbcConnection(connection)); | ||
| LockServiceFactory.getInstance().getLockService(database).forceReleaseLock(); | ||
| } catch (Exception e) { | ||
| log.error("Error occurred while releasing lock from Liquibase: {}", e.getMessage(), e); | ||
| } finally { | ||
| if (connection != null) { | ||
| try { | ||
| connection.close(); | ||
| } catch (SQLException e) { | ||
| log.error("Error occurred while closing connection: {}", e.getMessage(), e); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Let's use the 'mngtDataSource' already configured in the context. The simplified code would look like and of course handle the exceptions well.
// Add the applicationContext
if (applicationContext.containsBean("mngtDataSource")) {
DataSource mgtDatasource = applicationContext.getBean("mngtDataSource", DataSource.class);
try {
Database database = DatabaseFactory.getInstance().findCorrectDatabaseImplementation(new JdbcConnection(mgtDatasource.getConnection()));
LockServiceFactory.getInstance().getLockService(database).forceReleaseLock();
} catch (DatabaseException | SQLException | LockException e) {
throw new RuntimeException(e);
}
}There was a problem hiding this comment.
Should we add logic to close connection after lock is released?
finally {
if (connection != null) {
try {
connection.close();
} catch (SQLException e) {
log.error("Error occurred while closing connection: {}", e.getMessage(), e);
}
}
| # ---- Liquibase Configuration ----------------------------------------------------------------------------------------- | ||
| spring.liquibase.enabled=${ENABLE_LIQUIBASE} |
There was a problem hiding this comment.
How are you utilizing this property in your code?
There was a problem hiding this comment.
This is required to prevent liquibase from searching changelog.xml files and returning error for eip-demo service
There was a problem hiding this comment.
I think we can do away with this property since it's already been set here.
There was a problem hiding this comment.
It is required otherwise the eip-demo service fails to start
JIRA: https://mekomsolutions.atlassian.net/browse/OZ-532
Added liquibase dependency & code to force release lock when DB url is set (check is required because eip-demo doesn't depend on liquibase)