MLE-12345 Merge release/8.0.0 into master #1847
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MLE-12345 Merge master to develop and bump version
Not sure how it's reporting on files under ".kotlin", so hopefully the gitignore addition will prevent that.
MLE-12345 Polaris fixes
MLE-23032 Bumping to Java 17 and OkHttp 5
MLE-23032 Fixing Jenkins build for Java 21
This is a prototype; we don't want to apply it automatically. Intent for now is to see if this helps avoid connection errors during the regression piplines in Jenkins.
MLE-23230 Trying out retry interceptor
MLE-23230 Tweaking retry interceptor
This avoids hardcoding it in the actual client and let's us still see the results for the tests. Also fixed a warning from Polaris about the interceptor.
This internal API had several unused methods. In addition, application of the client configurators happens in OkHttpServices now. That removes as much OkHttp stuff as possible from DatabaseClientFactory. This will greatly simplify shifting to the JDK HTTP client some time in the future.
Trying to get as many okhttp3 imports into one package as possible, with the notable exception for now of OkHttpServices.
This was already in the public API via the Transaction interface, but it was in the "impl" package. I also can't determine any reason why it was reconstructing an OkHttp Cookie and then holding onto it. The Cookie class is immutable - but ClientCookie was already being given the immutable values that it needed. So no idea why ClientCookie was then rebuilding an OkHttp Cookie. It no longer does that - it's just a simple bean now, holding onto the values of interest that were extracted from an OkHttp Cookie.
Should make Black Duck happy as well. Not touching the Jakarta APIs yet, going to take care of that in a follow up PR as that needs more testing.
The javax.ws.rs-api dependency was only needed for a collection class that wasn't needed. Also forcing the usage of commons-lang3 as Black Duck mysteriously thinks an instance of 3.7 is being brought in as a direct dependency somehow. Also switched to latest nightly build for local testing, as Jenkins is using that too.
Copilot missed this one
I think there's more to be done here - haven't chatted with Copilot yet - but this at least gets things looking very similar to our other repos that use maven-publish.
This avoids Gradle warnings and also gets the reverse proxy server tests working properly again. Also disabling the reverse proxy tests, as those apparently were not hitting the reverse proxy before due to a Gradle misconfiguration that this PR fixes. Opened MLE-24523 to fix those later.
Eliminating more warnings, and forcing an error on a compiler warning. Got rid of two systemProperty values that I don't think are needed, but we'll see what happens in the full test run.
No warnings or errors locally. Also shifting to the 11.3.2 build temporarily to try things out. And fixing a problem with the ml-development-tools tests.
The Bitemp test is failing due to an ml-gradle bug, but I took the opportunity to clean it up so that each test can run separately without depending on the other.
And added comments to explain what the existing app-level retry support is doing vs what this for-now-undocumented interceptor will be doing.
Verifying that all code can be compiled, even when running a subset of the tests.
Also formatted the file so it's pretty. Only functional change though is on line 181, which ensures that the stage still completes even if tests fail.
Lots of little improvements to Jenkinsfile too.
I had this happen locally today, and clearing okhttp from my local Maven cache fixed the problem. No idea what's going wrong here, but it happened in the regression jobs.
Some progress, but 87 errors still. Adding those to Jira ticket.
Bumped Kotlin in ml-development-tools to be consistent with the Kotlin used by OkHttp 5.2.0
Also eagerly bumped up the NOTICE file.
There was a problem hiding this comment.
Pull Request Overview
This pull request merges the release/8.0.0 branch into master, upgrading the MarkLogic Java Client API to version 8.0.0. The release includes major dependency upgrades, requires Java 17 as the minimum version, and introduces new retry functionality while removing deprecated APIs.
Key changes include:
- Upgrade from Java 8/11 to Java 17 minimum requirement
- Major dependency upgrades including OkHttp 5.2.0, Jackson 2.20.0, and Jakarta JAXB 4.x
- New retry interceptor for handling connection failures
Reviewed Changes
Copilot reviewed 53 out of 56 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle.properties | Version bump to 8.0.0 and dependency version updates |
| pom.xml | Dependency version updates and removal of deprecated JAX-RS dependency |
| marklogic-client-api/build.gradle | Build configuration updates for Java 17 and new publishing setup |
| Various Java source files | Refactoring, new retry functionality, and API cleanup |
| Test files | Test updates and fixes for new functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Comment on lines
+127
to
+129
| public record ConnectionConfig(String host, int port, String basePath, String database, | ||
| SecurityContext securityContext, List<OkHttpClientConfigurator> clientConfigurators) { | ||
| } |
There was a problem hiding this comment.
[nitpick] Consider adding parameter validation to the record constructor to ensure required fields are not null, especially for critical connection parameters like host and securityContext
Copilot uses AI. Check for mistakes.
Comment on lines
+68
to
+79
| private boolean isRetryableIOException(IOException e) { | ||
| return e instanceof ConnectException || | ||
| e instanceof SocketTimeoutException || | ||
| e instanceof UnknownHostException || | ||
| (e.getMessage() != null && ( | ||
| e.getMessage().contains("Failed to connect") || | ||
| e.getMessage().contains("unexpected end of stream") || | ||
| e.getMessage().contains("Connection reset") || | ||
| e.getMessage().contains("Read timed out") || | ||
| e.getMessage().contains("Broken pipe") | ||
| )); | ||
| } |
There was a problem hiding this comment.
[nitpick] The string-based exception message matching is fragile and could break with different locales or JVM implementations. Consider using more specific exception types or documenting the assumptions about exception messages
Copilot uses AI. Check for mistakes.
rjrudin
changed the title
BillFarber
approved these changes
Oct 9, 2025
BillFarber
approved these changes
Oct 10, 2025
rjrudin
force-pushed
the
release/8.0.0
branch
from
0f9b84e to
af2b8d3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No real functional changes, just upgrading dependencies and now requiring Java 17.