-
Notifications
You must be signed in to change notification settings - Fork 4
Improved CI workflow to build, test (matrix), push server Docker img #2
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
Conversation
) * Improved CI workflow to build, test (matrix), push server Docker img * Merged jobs server and driver * Removed modules from matrix to test all at once * Fixes build/test matrix execution Fixes ojp-server build step by including missing modules (#5) chore: lint ojp-server pom file Moved ojp-server image build job to different workflow Fixes main.docker.yml incorrect job ci: added delay for server to start, fixed server docker settings
.github/workflows/main-docker.yml
Outdated
| -Dimage="${DOCKERHUB_REPO}:latest" \ | ||
| -Djib.to.auth.username="${DOCKERHUB_USER}" \ | ||
| -Djib.to.auth.password="${DOCKERHUB_TOKEN}" \ | ||
| -Djib.to.image="${DOCKERHUB_REPO}:latest" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to keep the repository and image name hard-coded here. I use env vars to test with my own DockerHub account.
Another thing you may want to change is the tag. I set it to latest, but I saw that you are creating as 0.0.1-alpha
Let me know if you want to change this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this to make only DockerHub repository configurable through Action vars.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
version is 0.0.3-alpha, we are not using latest for now to not look like it is a production ready version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repurposed the workflow to be dispatched manually.
You can run the workflow by providing tag (default: latest) and repository (default: rrobetti)
…extra fixes of issues found during draft performance testing.
# Conflicts: # .github/workflows/main.yml # README.md # ojp-grpc-commons/pom.xml # ojp-jdbc-driver/pom.xml # ojp-jdbc-driver/src/main/java/org/openjdbcproxy/jdbc/Driver.java # ojp-jdbc-driver/src/main/java/org/openjdbcproxy/jdbc/PreparedStatement.java # ojp-jdbc-driver/src/main/java/org/openjdbcproxy/jdbc/ResultSet.java # ojp-jdbc-driver/src/main/java/org/openjdbcproxy/jdbc/ResultSetMetaData.java # ojp-server/dependency-reduced-pom.xml # ojp-server/pom.xml # pom.xml
| @Override | ||
| public double getDouble(String columnLabel) throws SQLException { | ||
| return this.getDouble(this.labelsMap.get(columnLabel.toUpperCase())); | ||
| return this.getDouble(this.labelsMap.get(columnLabel.toUpperCase()) + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After rebasing I noticed this was missing.
|
@rrobetti The stress test |
) * Improved CI workflow to build, test (matrix), push server Docker img * Merged jobs server and driver * Removed modules from matrix to test all at once * Fixes build/test matrix execution Fixes ojp-server build step by including missing modules (#5) chore: lint ojp-server pom file Moved ojp-server image build job to different workflow Fixes main.docker.yml incorrect job ci: added delay for server to start, fixed server docker settings
* ci: fixes server module build JDK setup * fix: instanceof in ResultSet incompatibility
rrobetti
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work Roger.
Change log
Improves current "Main CI" workflow:
Modules ojp-grpc-commons and ojp-jdbc-driver are tested using Java 11, 17, 21 and 22
Module ojp-server is tested using Java 22 only
Refactored commons and driver for retro-compatibility
Modules can be compiled from Java 11-22
Most changes related to this refactory was replacing toList API from Java Stream with ArrayList and Java String blocks that are only available from Java 21.
Added new workflow for OJP Server Docker image build
See .github/workflows/main-docker.yml
This workflow only triggers when merged to main and changes made within common and server modules
It requires repository secrets
Vars (perhaps we can keep this hard coded?)