TRUNK-6058 : Create CI Process to create liquibase snapshots #4474

ManojLL · 2023-11-30T02:32:40Z

Description of what I changed

I write a script to automate the liquibase snapshot generating process

Issue I worked on

see https://issues.openmrs.org/browse/TRUNK-6058

Checklist: I completed these to help reviewers :)

My IDE is configured to follow the code style of this project.

No? Unsure? -> configure your IDE, format the code and add the changes with git add . && git commit --amend
I have added tests to cover my changes. (If you refactored
existing code that was well tested you do not have to add tests)

No? -> write tests and add them to this commit git add . && git commit --amend
I ran mvn clean package right before creating this pull request and
added all formatting changes to my commit.

No? -> execute above command
All new and existing tests passed.

No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.
My pull request is based on the latest changes of the master branch.

No? Unsure? -> execute command git pull --rebase upstream master

ManojLL · 2023-11-30T02:34:10Z

@dkayiwa @ibacher could you please review this work?

dkayiwa · 2023-12-01T12:41:25Z

@ManojLL as per this https://wiki.openmrs.org/display/docs/Pull+Request+Tips you need to transition the JIRA ticket state to indicate that you are waiting for review.

dkayiwa · 2023-12-01T12:42:52Z

Are you able to run this on our bamboo CI server?

ManojLL · 2023-12-02T05:29:22Z

Are you able to run this on our bamboo CI server?

This has not integrate with CI process yet, I just need to review my work and then we can run this in CI process.

dkayiwa · 2023-12-02T20:03:15Z

How will you know that it works if you have not tested it out?

ManojLL · 2023-12-07T04:27:14Z

@dkayiwa here are the steps to generate liquilbase snapshot automatically

set ENV variable called OPENMRS_INSTALLATION_SCRIPT
export OPENMRS_INSTALLATION_SCRIPT=<path to openmrs-core folder>/liquibase/scripts/installation.properties
run the script, username and passowrd refer to a MySQL user:
cd <path to openmrs-core folder>
. liquibase/scripts/generate_liquibase_snapshots.sh <username> <password>

dkayiwa · 2023-12-07T12:42:30Z

Don't i need to set some sort of permissions to make the script an executable?
Can we also avoid running the tests?

dkayiwa · 2023-12-07T12:53:38Z

liquibase/scripts/generate_liquibase_snapshots.sh

+ # Change directory to the 'webapp' folder
+ cd webapp
+
+ # Remove the ~/.OpenMRS directory


Do you really want to remove the .OpenMRS folder and i lose my existing OpenMRS installation?

This is what I was thinking about,
I just trying to automate the snapshot generating steps mentioned here. However, we can not drop existing can't delete our current production-level database and application while doing this. so alter this scrip to generate the snapshots without dropping current database and existing application data and this worked successfully and generate liquibase snapshots without any issue.

I'm not quite sure about the generated snapshots are the same when using these two methods.

Your script wipes out all the existing data that existing openmrs instances store in the application data directory. Can you avoid that?

dkayiwa · 2023-12-07T12:56:44Z

liquibase/scripts/installation.properties

+install_method=auto
+connection.url=jdbc:mysql:https://localhost:3306/@DBNAME@?autoReconnect=true&sessionVariables=default_storage_engine=InnoDB&useUnicode=true&characterEncoding=UTF-8
+connection.driver_class=com.mysql.cj.jdbc.Driver
+connection.username=root


As per the instructions you gave, i do not see how the username and password is passed over.
No wonder i got Access denied for user 'root'@'localhost' (using password: YES) when i tried to run this in spite of the fact that i supplied a correct username and password.

I just realized I missed mentioning a couple of instructions earlier, my bad!

I alter the script, so if this works fine we do not meed to run the installation process again.

I just realized I missed mentioning a couple of instructions earlier, my bad!

So where are you putting the instructions?

@ManojLL did you see the above question?

this is where I put the instructions for run the script. what I forgot to mention is chmod +x <path to script>.

Please do not keep instructions in a github comment.

Did you address the above comment?

@dkayiwa Do I need to update liquibase read me file with the instructions ?

@dkayiwa I added the instructions to the Readme.md file.

ManojLL · 2023-12-07T19:17:00Z

Can we also avoid running the tests? yes we can , when make a release we run the tests so I think we do not need to run the test again in the script.

dkayiwa · 2023-12-07T23:02:41Z

So can you make the necessary changes?

ManojLL · 2023-12-08T03:41:43Z

So can you make the necessary changes?

Update the script to generate snapshot without dropping the existing database.

dkayiwa · 2023-12-08T11:16:23Z

@ManojLL is this still Draft?

dkayiwa · 2023-12-08T11:19:10Z

liquibase/scripts/generate_liquibase_snapshots.sh

+}
+
+project_version=$(grep -m 1 '<version>' pom.xml | sed -n 's/.*<version>\(.*\)<\/version>.*/\1/p') 
+openMRS_version=$(echo $project_version | sed "s/\([0-9]*\.[0-9]*\)\..*/\1/")


Is there a reason why for some you are using capital letters and then others lower case? openMRS_version vs new_openmrs_version

No reason to name like this, I will correct the variable names

dkayiwa · 2023-12-08T11:21:39Z

liquibase/scripts/generate_liquibase_snapshots.sh

+
+ # Update the version in openmrs_version.txt only if it's a new version
+ if [ "${openMRS_version}" != "${new_openmrs_version}" ]; then
+ sed -i "s/\(openmrs_version.txt=\).*/\1${new_openmrs_version}.0-SNAPSHOT/" "liquibase/scripts/fix_liquibase_snapshots.sh"


Did you really test the line above?

This is not required now, missed to remove this line from the script when ,I will remove these lines

dkayiwa · 2023-12-09T22:18:17Z

liquibase/scripts/generate_liquibase_snapshots.sh

+ cp "api/src/main/resources/liquibase-update-to-latest-template.xml" "api/src/main/resources/org/openmrs/liquibase/updates/$new_liquibase_file"
+
+ database_updater_class="api/src/test/java/org/openmrs/util/DatabaseUpdaterDatabaseIT.java"
+ variable_name="CHANGE_SET_COUNT_FOR_GREATER_THAN_2_1_X"


Why are we explicitly using 2.1.x here?

according to the steps mentioned here, we are copying the liquibase-updates template and rename it to current version. That template file contains change set as below (this include a comment),
<changeSet id="42ddb873-f064-4418-a6c0-6c09426c4832" author="wschlegel"> <comment>Empty change set for integration tests.</comment> </changeSet>

So when adding a new change-set we need to increase the CHANGE_SET_COUNT_FOR_GREATER_THAN_2_1_X value by one.

But the current version is not 2.1.?

when adding new changeset in a new version which is greater than 2.1, we should increase this CHANGE_SET_COUNT_FOR_GREATER_THAN_2_1_X value by the number of newly added changesets.

dkayiwa · 2023-12-09T22:18:32Z

liquibase/scripts/generate_liquibase_snapshots.sh

+ sed -i "s/\($variable_name *= *\)[0-9]\+/\1$(( $(grep -o "$variable_name *= *[0-9]\+" "$database_updater_class" | awk '{print $NF}') + 1 ))/" "$database_updater_class"
+
+ insert_line=" <include file=\"org/openmrs/liquibase/updates/$new_liquibase_file\"/>"
+ sed -i -e '$!N;$!N;$i\'"$insert_line" "api/src/main/resources/liquibase-update-to-latest-from-1.9.x.xml"


Why are we using 1.9.x here?

if the liquibase snapshot update version is above 1.9.x , we add that names in this file. This also mentioned in here

In the documentation that you referenced above, isn't 1.9.x just an example. Where the actual value depends on the current version?

The file called liquibase-update-to-latest-from-1.9.x.xml is used for integration tests. When we make a new snapshot, we create a file for the next snapshot version using the format liquibase-update-.x.xml. After creating this new file, we need to add its path to the liquibase-update-to-latest-from-1.9.x.xml file.

dkayiwa · 2024-01-29T18:33:48Z

liquibase/README.md

+#### step 2 - run the script `openmrs/liquibase/scripts/generate_liquilbase_snapshots.sh`:
+Run the following commands to generate the Liquibase snapshots automatically, where username and password refer to a MySQL user:
+
+ . liquibase/scripts/generate_liquibase_snapshots.sh <username> <password>


After running this script, can you list the files which will get created and their output locations?

@dkayiwa I updated the read.me file by adding the details of created and updated files, along with their output locations.

TRUNK-6058 : kill the process after installation completing TRUNK-6058 : added logs TRUNK-6058 : remove drop database process TRUNK-6058 : delete installation.properties file TRUNK-6058 : remove unwanted line TRUNK-6058 : remove unwanted line TRUNK-6058 : update script TRUNK-6058 : update script TRUNK-6058 : update script TRUNK-6058 : update script TRUNK-6058 : change project version TRUNK-6058 : change project version TRUNK-6058: fixed the new version issue when generating liquibase snapshots TRUNK-6058: fixed the new version issue when generating liquibase snapshots TRUNK-6058: added instructions to run the script TRUNK-6058: update the Read.me file

ManojLL marked this pull request as draft December 2, 2023 05:30

dkayiwa reviewed Dec 7, 2023

View reviewed changes

ManojLL force-pushed the TRUNK-6058 branch from 416b66c to 1d1fd81 Compare December 8, 2023 03:39

dkayiwa reviewed Dec 8, 2023

View reviewed changes

ManojLL force-pushed the TRUNK-6058 branch from 1d1fd81 to 8071425 Compare December 8, 2023 14:56

ManojLL marked this pull request as ready for review December 8, 2023 14:57

dkayiwa reviewed Dec 9, 2023

View reviewed changes

ManojLL force-pushed the TRUNK-6058 branch from 8071425 to 56be43c Compare December 13, 2023 13:52

ManojLL force-pushed the TRUNK-6058 branch from 56be43c to c0bfbdd Compare January 3, 2024 11:53

ManojLL force-pushed the TRUNK-6058 branch from c0bfbdd to 6a346c5 Compare January 15, 2024 04:38

dkayiwa reviewed Jan 29, 2024

View reviewed changes

ManojLL force-pushed the TRUNK-6058 branch from 6a346c5 to 8603b98 Compare January 30, 2024 18:58

TRUNK-6058 : Create CI Process to create liquibase snapshots #4474

Are you sure you want to change the base?

TRUNK-6058 : Create CI Process to create liquibase snapshots #4474

Conversation

ManojLL commented Nov 30, 2023

Description of what I changed

Issue I worked on

Checklist: I completed these to help reviewers :)

ManojLL commented Nov 30, 2023

dkayiwa commented Dec 1, 2023

dkayiwa commented Dec 1, 2023

ManojLL commented Dec 2, 2023

dkayiwa commented Dec 2, 2023

ManojLL commented Dec 7, 2023

dkayiwa commented Dec 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkayiwa Dec 7, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ManojLL commented Dec 7, 2023

dkayiwa commented Dec 7, 2023

ManojLL commented Dec 8, 2023

dkayiwa commented Dec 8, 2023

Choose a reason for hiding this comment

ManojLL Dec 8, 2023 • edited

Choose a reason for hiding this comment

dkayiwa Dec 8, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkayiwa Dec 7, 2023 •

edited

ManojLL Dec 8, 2023 •

edited

dkayiwa Dec 8, 2023 •

edited