Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OZ-462: Ozone distro to repackage the O3 frontend #42

Merged
merged 6 commits into from
Feb 16, 2024

Conversation

ibacher
Copy link
Contributor

@ibacher ibacher commented Feb 7, 2024

This is an attempt to fulfill OZ-462. I have not yet added docs, as there are some details I'm hoping to finalize.

Assumptions:

  1. Machines running the distro have a recentish version Node / NPM (I think v16+) installed and available on the system PATH.
  2. Ozone deployments do not actually need the customizations provided by spa-build-config.json.

How this works:

  • Ozone now downloads the org.openmrs.distro:referenceapplication-frontend zip file and unpacks it in $OZONE_DIR/configs/openmrs/frontend_assembly/reference-application-spa-assemble-config.json
  • An Ozone distribution can provide a custom spa-assemble-config.json file in $DISTRO_ROOT/configs/openmrs/frontend_assembly that specifies their additions and removals.
  • In an Ozone distribution, the reference-application-spa-assemble-config.json is copied to ${project.build.directory}/openmrs_frontend and the spa-assemble-config.json file (if any) is copied to ${project.build.directory}/openmrs_frontend.
  • If a spa-assemble-config.json file is found, then the assemble (and, for now, at least, build) commands will be run. The outputs of this are published to /distro/binaries/openmrs/frontend/.

Open questions:

  • Are these files in the right places or should they go somewhere else? (Little hard to know what to do with the assemble-configs; they aren't traditional config files in that they aren't part of the runtime, but they also aren't binaries for the same reason).
  • Are the assumptions here ok?
  • Do we need to always have some kind of output in /distro/binaries/openmrs/frontend/, e.g., so that this works with Docker?

(Apologies for adding yet more Groovy; I couldn't figure out how to do the necessary scripting in Ant)

@ibacher
Copy link
Contributor Author

ibacher commented Feb 7, 2024

@ibacher ibacher marked this pull request as draft February 7, 2024 20:28
@rbuisson
Copy link
Contributor

rbuisson commented Feb 8, 2024

Do we need to always have some kind of output in /distro/binaries/openmrs/frontend/, e.g., so that this works with Docker?

Let's have the OpenMRS frontend always being re-assembled by Ozone even though based on default Ref App assemble JSON. That way we can always assume binaries will be present in the /distro/binaries/openmrs/frontend/ folder.

@rbuisson
Copy link
Contributor

rbuisson commented Feb 8, 2024

Are the assumptions here ok?

As discussed in the call, we would want to be able to override the SPA_CONFIG_URLS and DEFAULT_LOCALE, but it looks like you've already pushed a fix for that. 👍🏻 .

@rbuisson
Copy link
Contributor

rbuisson commented Feb 8, 2024

Are these files in the right places or should they go somewhere else? (Little hard to know what to do with the assemble-configs; they aren't traditional config files in that they aren't part of the runtime, but they also aren't binaries for the same reason).

I still hesitant on this one.
Let's keep it in $OZONE_DIR/configs/openmrs/frontend_assembly/ and even keep it in the final package.
Even though it has no value at runtime, it's a very small file and won't hurt much.

@ibacher
Copy link
Contributor Author

ibacher commented Feb 8, 2024

@rbuisson Another two questions (from the latest commit): I externalized the Groovy from the version embedded in the POM to an actual file, with the idea that it could be shared from Ozone to Ozone implementations and moved this to scripts/openmrs/frontend_assembly (though on reflection, it's probably better to go with scripts/openmrs/frontend).

  1. Is that a reasonable place to put it?
  2. Is it ok to publish scripts as part of the Ozone distro? (If so, I would probably use this same thing in the archetype to avoid the duplication of scripts)

@rbuisson
Copy link
Contributor

rbuisson commented Feb 9, 2024

@ibacher ,

Is it ok to publish scripts as part of the Ozone distro? (If so, I would probably use this same thing in the archetype to avoid the duplication of scripts)

"Ozone distro" what package exactly? The final ozone? or ozone-distro?
OK to publish it but would it be possible to have it as a separate Maven package: "scripts".
What do you think?

@rbuisson
Copy link
Contributor

rbuisson commented Feb 9, 2024

Is that a reasonable place to put it?

Works for me.

@ibacher
Copy link
Contributor Author

ibacher commented Feb 9, 2024

OK to publish it but would it be possible to have it as a separate Maven package: "scripts".

Oh, I like that idea a lot better. Let me refactor things again.

Yeah, I was meaning literally part of the ozone-distro artifact, but that's less than ideal.

@ibacher ibacher marked this pull request as ready for review February 12, 2024 17:54
@ibacher
Copy link
Contributor Author

ibacher commented Feb 12, 2024

So I've added a scripts project to Ozone that bundles up go-to-scripts-dir.sh, set-ozone-dir.sh and the Groovy script to build / rebuild the frontend. Ozone (the distro) will use it as-is. Implementation projects rely on it being unpacked into the target directory.

<artifactId>gmavenplus-plugin</artifactId>
<executions>
<execution>
<id>Rebuild OpenMRS Frontend if necessary</id>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be renamed to "Rebuild OpenMRS Frontend"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one only triggers if there are local customisations, but I can rename it.

<overwrite>true</overwrite>
<resources>
<resource>
<directory>${project.basedir}/configs/openmrs/frontend_assembly</directory>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this path correct?
I thought the frontend assembly config file was moved to scripts/.
Anyway, I don't see it in /configs/openmrs/frontend_assembly either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This step copies the implementation-specific spa-assemble-config.json file, if any. I didn't change that location, but I can if it makes more sense in scripts?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's OK we can leave it at this place.
But shouldn't this be distro/configs/openmrs/frontend_assembly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the implementation-specific projects, we usually don't have a distro sub-folder...

scripts/pom.xml Outdated Show resolved Hide resolved
@rbuisson
Copy link
Contributor

@ibacher , the frontend build seems to hang when running ./scripts/mvnw clean package.

[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO] 
[INFO] Ozone Maven Commons                                                [pom]
[INFO] Ozone Distribution                                                 [pom]
[INFO] Ozone                                                              [pom]
[INFO] Ozone Scripts                                                      [pom]
[INFO] Ozone Maven Parent                                                 [pom]
[INFO] 
[INFO] ---------------------< com.ozonehis:maven-commons >---------------------
[INFO] Building Ozone Maven Commons 1.0.0-SNAPSHOT                        [1/5]
[INFO]   from maven-commons/pom.xml
[INFO] --------------------------------[ pom ]---------------------------------
[INFO] 
[INFO] --- clean:3.2.0:clean (default-clean) @ maven-commons ---
[INFO] 
[INFO] ---------------------< com.ozonehis:ozone-distro >----------------------
[INFO] Building Ozone Distribution 1.0.0-SNAPSHOT                         [2/5]
[INFO]   from distro/pom.xml
[INFO] --------------------------------[ pom ]---------------------------------
[INFO] 
[INFO] --- clean:3.2.0:clean (default-clean) @ ozone-distro ---
[INFO] Deleting /Users/rom/repos/ozone/distro/target
[INFO] 
[INFO] --- dependency:3.6.1:unpack-dependencies (Copy Odoo Initializer add-on) @ ozone-distro ---
[INFO] 
[INFO] --- dependency:3.6.1:copy-dependencies (Copy OpenMRS modules to a temporary location) @ ozone-distro ---
[INFO] Copying patientsummary-omod-2.2.0.jar to /Users/rom/repos/ozone/distro/target/openmrs_modules/patientsummary-omod-2.2.0.jar
[INFO] Copying commonreports-omod-1.4.0-SNAPSHOT.jar to /Users/rom/repos/ozone/distro/target/openmrs_modules/commonreports-omod-1.4.0-SNAPSHOT.jar
[INFO] 
[INFO] --- dependency:3.6.1:unpack-dependencies (Unpack OpenMRS Ref App in a temporary location) @ ozone-distro ---
[INFO] 
[INFO] --- dependency:3.6.1:unpack-dependencies (Unpack OpenMRS Ref App frontend configuration) @ ozone-distro ---
[INFO] 
[INFO] --- antrun:3.1.0:run (Rename spa-assemble-config.json to reference-application-spa-assemble-config.json) @ ozone-distro ---
[INFO] Executing tasks
[INFO]      [copy] Copying 1 file to /Users/rom/repos/ozone/distro/target/ozone-distro-1.0.0-SNAPSHOT/configs/openmrs/frontend_assembly
[INFO]      [move] Moving 1 file to /Users/rom/repos/ozone/distro/target/openmrs_frontend
[INFO] Executed tasks
[INFO] 
[INFO] --- resources:3.3.1:copy-resources (Exclude some OpenMRS Ref App files) @ ozone-distro ---
[INFO] Copying 71 resources from target/referenceapplication-distro/openmrs_config to target/openmrs_config
[INFO] 
[INFO] --- antrun:3.1.0:run (Rename JAR to OMOD) @ ozone-distro ---
[INFO] Executing tasks
[INFO]      [move] Moving 2 files to /Users/rom/repos/ozone/distro/target/openmrs_modules
[INFO] Executed tasks
[INFO] 
[INFO] --- gplus:3.0.2:execute (Build OpenMRS Frontend) @ ozone-distro ---
[INFO] Using plugin classloader, includes GMavenPlus and project classpath.
[INFO] Using Groovy 4.0.18 to perform execute.
[INFO] Running Groovy script from file:/Users/rom/repos/ozone/distro/../scripts/openmrs/frontend_assembly/build-openmrs-frontend.groovy.
[INFO] Checking for frontend customizations in /Users/rom/repos/ozone/distro/target/openmrs_frontend/spa-assemble-config.json
[INFO] Project: com.ozonehis:ozone-distro
[INFO] Running assemble command...
npx: installed 695 in 12.414s
[09:27:16] using cwd ~/repos/ozone
[09:27:16] updated package.json config with {}
[09:27:16] starting assemble 
[09:27:16] starting assemble:default task
[09:27:16] starting assemble:prompt-new task

and it hangs here.

@ibacher
Copy link
Contributor Author

ibacher commented Feb 14, 2024

@rbuisson What version of Node do you have installed? (See the assumptions in the initial post of this PR). (I honestly have never seen any of the messages after the [INFO] Running assemble command... line).

@rbuisson
Copy link
Contributor

Yep. What using a 17.x. Works fine with 18.x.

@ibacher
Copy link
Contributor Author

ibacher commented Feb 14, 2024

Yep. What using a 17.x. Works fine with 18.x.

Ah... I should revise the comment. We support the LTS versions of Node, which honestly means "the even version numbers" (Node's versioning policy is that every other major release is LTS). I tested it with 16 and 18, but not 17.

@rbuisson rbuisson merged commit 1a95184 into ozone-his:main Feb 16, 2024
3 checks passed
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