OBPIH-6354 Bug resolving upgrade changelog versions #4601

jmiranda · 2024-04-25T04:18:18Z

No description provided.

jmiranda · 2024-04-25T04:18:50Z

build.gradle

@@ -123,7 +123,7 @@ assets {

 bootRun {
    addResources = true
-    dependsOn 'npm_run_bundle'
+    //dependsOn 'npm_run_bundle'


Ignore for now, I just didn't want to wait around for this shit while I was running tests.

Apologies in advance but we don't have a place to put these tangential discussions so I'm adding my thoughts here until I can find a good place for this.

@awalkowiak (or anyone else who might know) Is it possible to remove "npm run bundle" when using run-app or debug-app. I think we need to execute it for packaging the WAR file, but when running the application in developer we can start the React / webpack command ourselves.

In fact, if I was a developer I'd probably prefer to use

npm run watch

on its own.

However, I know without the npm_run_bundle the React application won't be available or not having it bundled each time will cause confusion if we're using older resources. But the start is so painful I'd be ok with it.

A few followup questions and comments

Is there a good reason to keep the npm_run_bundle dependency on bootRun i.e. grails run-app?

Is there a way for us to run the "dependsOn" process in the background?

Can we add a placeholder bundle that writes to an error message to the DOM or an alert stating "The app is unavailable - please execute npm run watch in order to access the UI"

We could probably add a command line argument (property) to enable/disable webpack mode (i.e. bundle, watch, none) with none being the default

gradlew bootRun -Pmode=watch

We'd also want to figure out the equivalent when running via Grails since I don't think Grails passes arguments to gradle by default. We would need to explicitly capture and pass the arguments to Gradle.

grails run-app

Here's a resource that I think Artur probably encountered when working on the React integration a long time ago.
https://stackoverflow.com/questions/40348588/is-it-possible-to-integrate-react-using-webpack-with-grails

And at the bottom, there's a suggestion I don't love, but also don't hate re: npm scripts. It doesn't really help us unless we decided to fundamentally change the way we start the application in development. Although I guess having both options (heavy version using grails/gradle and light version using npm scripts) might be fine.
https://stackoverflow.com/a/40349256/136597

@jmiranda I had no chance to dive into this one more, perhaps this is a candidate for a discovery ticket?

I'm about to revert this change but wanted to get some data before doing so.

Can you all provide some metrics from your development environments (CPU usage, build time) when running "npm run bulde"

Without "npm run bundle" being executed, the build takes about 90 seconds and I can still use my computer while grails run-app command is starting up the application

With "npm run bundle", load is high and CPU is pegged at 100% for about 5 minutes making my laptop unuseable during that time. In fact, I can't even finish writing this message.

top - 12:00:17 up 2 days, 3:11, 1 user, load average: 9.34, 11.36, 8.01 Tasks: 476 total, 1 running, 475 sleeping, 0 stopped, 0 zombie %Cpu(s): 85.0 us, 6.1 sy, 0.0 ni, 7.6 id, 0.0 wa, 0.0 hi, 1.3 si, 0.0 st MiB Mem : 31730.5 total, 3986.2 free, 20225.6 used, 7518.7 buff/cache MiB Swap: 15737.5 total, 15630.6 free, 106.9 used. 10243.8 avail Mem PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND 269879 jmiranda 20 0 33.3g 1.1g 34336 S 569.2 3.7 4:53.21 webpack 269776 jmiranda 20 0 5192956 875540 35612 S 75.2 2.7 1:57.32 java 17317 jmiranda 20 0 1133.9g 448144 118096 S 27.5 1.4 358:23.45 Logseq 4396 jmiranda 20 0 5383544 817812 106552 S 12.9 2.5 116:05.66 gnome-shell 13755 jmiranda 20 0 33.1g 724856 300296 S 11.6 2.2 235:45.79 chrome 14246 jmiranda 20 0 1138.3g 273436 137364 S 10.6 0.8 43:57.59 chrome ```

I would like to know whether this is anomaly or whether others are experiencing this. If it's just me I'll keep commenting out npm_run_bundle on my own. If others are encountering this behavior, I'll create a discovery ticket to see if there's something we can do about it.

cc @EWaterman @kchelstowski @alannadolny @drodzewicz

I get a pretty decent spike up as well, though it doesn't totally brick my computer

jmiranda · 2024-04-25T04:19:25Z

build.gradle

@@ -304,6 +304,10 @@ configurations {
    }
 }

+configurations.integrationTestImplementation {
+    extendsFrom(configurations.implementation)
+}


Missing configuration to prevent ClassNotFoundErrors while running tests. Need to make sure this is the proper solution.

Does everyone feel comfortable with this?

¯\(ツ)/¯

jmiranda · 2024-04-25T04:20:44Z

build.gradle

@@ -609,7 +614,7 @@ dependencies {
    }

    // FIXME use commons-csv instead
-    implementation 'org.grails.plugins:excel-import:3.0.2'
+    compile 'org.grails.plugins:excel-import:3.0.2'


I need to revert this change, since the config above should fix this.

jmiranda · 2024-04-25T04:21:24Z

build.gradle

@@ -382,6 +386,7 @@ sourceSets {
 springBoot {
    buildInfo()
    mainClass = 'org.pih.warehouse.Application'
+    executable = true


Added this so that I could test the database migration with the application being deployed with standalone war i.e. embedded tomcat.

cd build/libs ./openboxes.war

We should also test to make sure the everything works properly when deployed to an actual Tomcat server.

It seems like this gives us the benefit of the creating an executable WAR file without the downside of dependency issues on the deployment side (i.e. embedded tomcat classes conflicting with the container classes installed on the server).

The WAR files are about the same size

-rwxrw-r-- 1 jmiranda jmiranda 187060158 May 15 12:09 openboxes-standalone.war -rw-rw-r-- 1 jmiranda jmiranda 187051977 May 15 12:12 openboxes.war

They both have the same number of files (and probably the same JARs)

[jmiranda@jmiranda-ThinkPad-W540 libs (OBPIH-6354-resolve-upgrade-version)]$ jar -tvf openboxes.war | wc -l 16141 [jmiranda@jmiranda-ThinkPad-W540 libs (OBPIH-6354-resolve-upgrade-version)]$ jar -tvf openboxes-standalone.war | wc -l 16141

The only difference seems to be in the way the files are "executable".

The "executable" WAR seems to have a bash script which can be seen if you head the file (see

[jmiranda@jmiranda-ThinkPad-W540 libs (OBPIH-6354-resolve-upgrade-version)]$ head -10 openboxes-standalone.war #!/bin/bash # # . ____ _ __ _ _ # /\\ / ___'_ __ _ _(_)_ __ __ _ \ \ \ \ # ( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \ # \\/ ___)| |_)| | | | | || (_| | ) ) ) ) # ' |____| .__|_| |_|_| |_\__, | / / / / # =========|_|==============|___/=/_/_/_/ # :: Spring Boot Startup Script :: #

According to the Spring Boot docs

Fully executable jars work by embedding an extra script at the front of the file. Currently, some tools do not accept this format, so you may not always be able to use this technique.

Source: https://docs.spring.io/spring-boot/docs/2.0.x/reference/html/deployment-install.html

This means that the executable WAR file can be executed like a binary file.

jmiranda@jmiranda-ThinkPad-W540 libs (OBPIH-6354-resolve-upgrade-version)]$ ./openboxes-standalone.war 12:14:19,807 |-INFO in ch.qos.logback.classic.LoggerContext[default] - Could NOT find resource [logback-test.xml] 12:14:19,807 |-INFO in ch.qos.logback.classic.LoggerContext[default] - Found resource [logback.xml] at [jar:file:/home/jmiranda/git/openboxes-grails-migration/build/libs/openboxes-standalone.war!/WEB-INF/classes!/logback.xml] 12:14:19,825 |-INFO in ch.qos.logback.core.joran.spi.ConfigurationWatchList@7d417077 - URL [jar:file:/home/jmiranda/git/openboxes-grails-migration/build/libs/openboxes-standalone.war!/WEB-INF/classes!/logback.xml] is not of type file

while the non-executable WAR can requires you to execute the WAR using java -jar

[jmiranda@jmiranda-ThinkPad-W540 libs (OBPIH-6354-resolve-upgrade-version)]$ java -jar openboxes.war 12:15:55,018 |-INFO in ch.qos.logback.classic.LoggerContext[default] - Could NOT find resource [logback-test.xml] 12:15:55,018 |-INFO in ch.qos.logback.classic.LoggerContext[default] - Found resource [logback.xml] at [jar:file:/home/jmiranda/git/openboxes-grails-migration/build/libs/openboxes.war!/WEB-INF/classes!/logback.xml] 12:15:55,038 |-INFO in ch.qos.logback.core.joran.spi.ConfigurationWatchList@7d417077 - URL [jar:file:/home/jmiranda/git/openboxes-grails-migration/build/libs/openboxes.war!/WEB-INF/classes!/logback.xml] is not of type file 12:15:55,224 |-INFO in ch.qos.logback.classic.joran.action.ConfigurationAction - debug attribute not set

So they are essentially the same, execute for that wrapper bash script. Do you think it's ok to keep it in or would you feel more comfortable if this was moved to a discovery ticket?

cc @EWaterman @awalkowiak @kchelstowski @alannadolny @drodzewicz

as long as you can still run it via java -jar (which it seems like you can), I don't have any issues with making it executable

jmiranda · 2024-04-25T04:22:30Z

grails-app/init/org/pih/warehouse/BootStrap.groovy

+            log.info("upgradeChangeLogVersions: " + upgradeChangeLogVersions)
+            if (upgradeChangeLogVersions?.empty) {
+                throw new RuntimeException("Unable to execute database migrations")
+            }


Not sure if this is the right approach (or error message) but I'd prefer to throw an error rather than silently failing like we have for the past few months.

Let's throw an error, but perhaps it would be better to give more descriptive reason? Especially why exactly "unable to execute"

The reason I wasn't more specific is because I honestly have no idea how to explain this. :)

Let me know if you have any thoughts.

jmiranda · 2024-04-25T04:23:47Z

src/main/groovy/util/LiquibaseUtil.groovy

+
+    static Set<String> getChangeLogVersions(String changeLogFile) {
+        Liquibase liquibase = new Liquibase(changeLogFile, new ClassLoaderResourceAccessor(), database)
+        println "changelogfile: " + liquibase.changeLogFile


Feeling cute, might delete later.

jmiranda · 2024-04-25T04:25:19Z

Integration tests seem to be running ok. At least when executed individually.

./gradlew integrationTest --tests util.LiquibaseUtilSpec --stacktrace

…on test

…unnecessary logging

…ainers dependency

…on flow

jmiranda · 2024-05-16T04:19:28Z

build.gradle

+        showStandardStreams = true
+        //exceptionFormat = TestExceptionFormat.FULL
+    }
+}


Does this look right?

https://docs.gradle.org/current/dsl/org.gradle.api.tasks.testing.logging.TestLogging.html

showStackTraces seems to be true by default

you can use the TestLogEvent enum in the events section if you don't want to hardcode strings

Otherwise it looks fine to me.

I found a fancier one but it's probably more than we need at this point:
https://stackoverflow.com/questions/3963708/gradle-how-to-display-test-results-in-the-console-in-real-time

OBPIH-6354 Bug resolving upgrade changelog versions

4c41923

jmiranda commented Apr 25, 2024

View reviewed changes

jmiranda requested review from alannadolny, awalkowiak, EWaterman, drodzewicz and kchelstowski April 25, 2024 04:25

awalkowiak added do not merge ready for review labels Apr 25, 2024

jmiranda added 3 commits April 26, 2024 20:38

OBPIH-6354 Added testcontainers dependencies and an initial integrati…

9d869a0

…on test

OBPIH-6354 Moved installation database migrations ahead of drop views

b52d062

OBPIH-6354 Minor change to test feature method to make it more clear

2ba2149

EWaterman mentioned this pull request May 2, 2024

OBPIH-5936 initial rest assured API tests #4600

Merged

OBPIH-6354 Reverted experimental changes to build.gradle; cleaned up …

0be5c9f

…unnecessary logging

jmiranda removed the do not merge label May 15, 2024

jmiranda added 2 commits May 15, 2024 17:42

OBPIH-6354 Removed integration tests used to experiment with testcont…

8ccaf95

…ainers dependency

OBPIH-6354 Improved comments in BootStrap to explain database migrati…

c822be8

…on flow

jmiranda commented May 16, 2024

View reviewed changes

awalkowiak changed the base branch from feature/upgrade-to-grails-3.3.10 to develop May 17, 2024 11:47

awalkowiak approved these changes May 21, 2024

View reviewed changes

Merge branch 'develop' into OBPIH-6354-resolve-upgrade-version

6d8fb40

awalkowiak merged commit 3eab73e into develop May 21, 2024
1 check passed

awalkowiak deleted the OBPIH-6354-resolve-upgrade-version branch May 21, 2024 08:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OBPIH-6354 Bug resolving upgrade changelog versions #4601

OBPIH-6354 Bug resolving upgrade changelog versions #4601

jmiranda commented Apr 25, 2024

jmiranda Apr 25, 2024

jmiranda Apr 25, 2024

awalkowiak Apr 26, 2024

jmiranda May 15, 2024

EWaterman May 16, 2024

jmiranda Apr 25, 2024

jmiranda May 15, 2024

EWaterman May 16, 2024 •

edited

jmiranda Apr 25, 2024

jmiranda Apr 25, 2024

jmiranda May 15, 2024

EWaterman May 16, 2024

jmiranda Apr 25, 2024

awalkowiak Apr 26, 2024

jmiranda May 16, 2024

jmiranda Apr 25, 2024

jmiranda commented Apr 25, 2024

jmiranda May 16, 2024

EWaterman May 16, 2024 •

edited

OBPIH-6354 Bug resolving upgrade changelog versions #4601

OBPIH-6354 Bug resolving upgrade changelog versions #4601

Conversation

jmiranda commented Apr 25, 2024

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

EWaterman May 16, 2024 • 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

jmiranda commented Apr 25, 2024

Choose a reason for hiding this comment

EWaterman May 16, 2024 • edited

Choose a reason for hiding this comment

EWaterman May 16, 2024 •

edited

EWaterman May 16, 2024 •

edited