Skip to content

STAND-121: Improve Windows Compatibility and Safe Data Directory Reuse for Standalone Platform#78

Merged
dkayiwa merged 34 commits intoopenmrs:masterfrom
Muta-Jonathan:Fix-windows
Jul 21, 2025
Merged

STAND-121: Improve Windows Compatibility and Safe Data Directory Reuse for Standalone Platform#78
dkayiwa merged 34 commits intoopenmrs:masterfrom
Muta-Jonathan:Fix-windows

Conversation

@Muta-Jonathan
Copy link
Copy Markdown
Member

@Muta-Jonathan Muta-Jonathan commented Jul 18, 2025

See https://openmrs.atlassian.net/browse/STAND-121

Description

This PR improves Windows support for the OpenMRS Standalone launcher by addressing issues related to MariaDB startup and file system behavior, particularly around data directory handling and platform-specific constraints.

It addresses On Windows, MariaDB startup fails with:
ERROR : Data directory ... is not empty. Only new or empty existing directories are accepted for --datadir

Screen Record on Windows

2025-07-18.07-49-49.mp4

Screen Record on Linux

Screencast.from.18-07-25.10.20.40.webm

Screen Record on Mac m2 Chip

Screen.Recording.2025-07-18.at.11.38.59.AM.mov

StandaloneUtil.resetConnectionPassword();
StandaloneUtil.startupDatabaseToCreateDefaultUser(mySqlPort);
Context.updateSearchIndex();
//Context.updateSearchIndex();
Copy link
Copy Markdown
Member Author

@Muta-Jonathan Muta-Jonathan Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented out these because they are crushing on Windows yet passing on linux and mac

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then why not completely remove?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay but this means the search Index will be updated by loading the war itself

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Muta-Jonathan
Copy link
Copy Markdown
Member Author

cc @dkayiwa @wikumChamith

import ch.vorburger.mariadb4j.DBConfigurationBuilder;

import java.io.File;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you document what this class is used for?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay

@Muta-Jonathan Muta-Jonathan requested a review from dkayiwa July 18, 2025 13:10
@dkayiwa
Copy link
Copy Markdown
Member

dkayiwa commented Jul 19, 2025

Did you test this on mac?

@Muta-Jonathan
Copy link
Copy Markdown
Member Author

Did you test this on mac?

Screenshot 2025-07-21 at 2 28 26 PM

Found out the previous builds were missing the demodatabase.zip now corrected and works

@Muta-Jonathan
Copy link
Copy Markdown
Member Author

cc @dkayiwa I think now this is ready for review

@Muta-Jonathan
Copy link
Copy Markdown
Member Author

Screenshot from 2025-07-21 14-34-22 on linux

@dkayiwa
Copy link
Copy Markdown
Member

dkayiwa commented Jul 21, 2025

@Muta-Jonathan can we also get rid of this error? java.sql.SQLSyntaxErrorException: Unknown column 'RESERVED' in 'WHERE' It makes the logs very noisy!

@Muta-Jonathan
Copy link
Copy Markdown
Member Author

Muta-Jonathan commented Jul 21, 2025

@dkayiwa thats possible however need to support the driver for mariadb here
https://github.com/openmrs/openmrs-core/blob/4a61d8a016decb61b89e09dcab16764698d6f0e1/api/src/main/java/org/openmrs/util/DatabaseUtil.java#L55-L82
which should fix it from the core side

However this means which fix will apply for 2.8 unless we include it also for 2.7 however that version is already out so the support will apply for 2.8.0

@dkayiwa
Copy link
Copy Markdown
Member

dkayiwa commented Jul 21, 2025

How would the fix look like? Do you have a pull request that i can review?

@dkayiwa
Copy link
Copy Markdown
Member

dkayiwa commented Jul 21, 2025

Can we also have the standalone window include the version of the platform or reference application (O2, O3) that we are running?

}

String os = System.getProperty("os.name").toLowerCase();
boolean isWindows = os.contains("win");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this is how we check for windows?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yes only if there is better way i can look into it

@dkayiwa
Copy link
Copy Markdown
Member

dkayiwa commented Jul 21, 2025

By the way, how are we going to deal with the search index problem? Because the standalone's demo mode option runs with no data for patients and concepts.

@dkayiwa
Copy link
Copy Markdown
Member

dkayiwa commented Jul 21, 2025

I am merging this because it fixes the windows problem. Let us fix the remaining issues using different tickets and pull requests.

@dkayiwa dkayiwa merged commit 911e40c into openmrs:master Jul 21, 2025
3 checks passed
@Muta-Jonathan
Copy link
Copy Markdown
Member Author

By the way, how are we going to deal with the search index problem? Because the standalone's demo mode option runs with no data for patients and concepts.

am gonna look into finding the better place to refresh it

@Muta-Jonathan
Copy link
Copy Markdown
Member Author

I am merging this because it fixes the windows problem. Let us fix the remaining issues using different tickets and pull requests.

oh yes

@dkayiwa
Copy link
Copy Markdown
Member

dkayiwa commented Jul 21, 2025

@Muta-Jonathan
Copy link
Copy Markdown
Member Author

oh yes Am wondering this passed locally and on PR but failing on merge

@Muta-Jonathan
Copy link
Copy Markdown
Member Author

let me tryto re-run it and see the output

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants