Skip to content

PECOBLR-1504 Update Github actions for multi module maven.#1180

Open
tejassp-db wants to merge 41 commits intoPECOBLR-1121/arrow-patch/stack-5from
PECOBLR-1121/arrow-patch/stack-6
Open

PECOBLR-1504 Update Github actions for multi module maven.#1180
tejassp-db wants to merge 41 commits intoPECOBLR-1121/arrow-patch/stack-5from
PECOBLR-1121/arrow-patch/stack-6

Conversation

@tejassp-db
Copy link
Collaborator

Update Github action workflows to run for multi module maven projects.

Update Github action workflows to run for multi module maven
projects.
@tejassp-db tejassp-db self-assigned this Jan 14, 2026

- name: Check Unit Tests
shell: bash
run: mvn test -Dtest='!**/integration/**,!**/DatabricksDriverExamples.java,!**/ProxyTest.java,!**/LoggingTest.java,!**/SSLTest.java'
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are we removing all these from here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The exclusions from the mvn test command were redundant — they're already excluded in the maven-surefire-plugin section in jdbc-core/pom.xml (which was copied over from the original pom.xml)

MAVEN_CENTRAL_USERNAME: ${{ secrets.MAVEN_CENTRAL_USERNAME }}
MAVEN_CENTRAL_PASSWORD: ${{ secrets.MAVEN_CENTRAL_PASSWORD }}

- name: Get tag name from commit
Copy link
Collaborator

Choose a reason for hiding this comment

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

It won't be triggered by new tag now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The new push: tags: 'v*' trigger does fire on new tags — so yes, it will still be triggered automatically. The difference is that the old workflow chained off the "Release" (uber JAR) workflow via workflow_run, meaning the thin JAR release only ran after the uber JAR release completed. Now both release workflows trigger independently on the same tag push. Since the multi-module setup builds the thin JAR from assembly-thin which depends on jdbc-core (built locally in Step 1), there's no dependency on the uber JAR release anymore — each workflow is self-contained.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted to manual trigger.

Copy link
Collaborator

@madhav-db madhav-db left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple of small questions. Thanks!

uses: actions/checkout@v4
with:
# If manual trigger: use input tag, else use the SHA from the completed workflow run
ref: ${{ github.event_name == 'workflow_dispatch' && format('refs/tags/{0}', inputs.tag) || github.event.workflow_run.head_sha }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did we remove this? Does github.ref point to a tag that can be selected during a manual trigger?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I made it an automated trigger.


jobs:
publish-thin:
# Only run if main JAR workflow succeeded or manual trigger
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we removed this, this means that the thin jar release doesn't depend on the main jar workflow success, right? This means, it's possible that the main workflow fails, and the thin jar one succeeds, effectively publishing just a thin jar on maven without publishing the main one. Is that something we're okay with?

Copy link
Collaborator Author

@tejassp-db tejassp-db Feb 12, 2026

Choose a reason for hiding this comment

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

Valid point, let me revert it to the earlier trigger mechanism.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed. Reverted to manual trigger.

key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
restore-keys: ${{ runner.os }}-m2

- name: Set up Maven Toolchains
Copy link
Collaborator

Choose a reason for hiding this comment

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

This step is already done above, duplicated

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nevermind, I see that we probably need it again. Can we abstract this? Otherwise looks good.

Copy link
Collaborator

@gopalldb gopalldb left a comment

Choose a reason for hiding this comment

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

Let's monitor this for few releases

tejassp-db and others added 9 commits March 2, 2026 12:20
## Description
<!-- Provide a brief summary of the changes made and the issue they aim
to address.-->
Replace Log4j2 dependencies with the SLF4J-to-JUL bridge (slf4j-jdk14)
so
that the fat jar can emit SDK and Apache HTTP client logs via Java Util
Logging (JUL) without requiring users to supply additional logging
libraries.

- Simplify JulLogger implementation to delegate directly to
java.util.logging
- Remove log4j-core, log4j-api, and log4j-slf4j2-impl test dependencies
- Remove Log4j2 shade plugin transformers from pom.xml
- Restructure docs/LOGGING.md with a fat jar vs thin jar quick-reference
table, JUL configuration examples, and SLF4J/Log4j2/Logback integration
guides
- Simplify logging section in README.md to point to detailed docs

## Testing
<!-- Describe how the changes have been tested-->
- Unit tests
- E2e tests

## Additional Notes to the Reviewer
<!-- Share any additional context or insights that may help the reviewer
understand the changes better. This could include challenges faced,
limitations, or compromises made during the development process.
Also, mention any areas of the code that you would like the reviewer to
focus on specifically. -->
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.

4 participants