Automatic module names - #865
Conversation
`openai-java-bedrock` shipped `BedrockOpenAIOkHttpClient` in the
`com.openai.client.okhttp` package, which is owned by
`openai-java-client-okhttp`. Because the Bedrock artifact declares a
compile-scope dependency on the OkHttp artifact, every consumer has both
JARs on the path, so the two always split that package.
On the class path this is harmless, but it makes the Bedrock artifact
unusable on the module path:
java.lang.module.ResolutionException: Modules com.openai.client.okhttp
and com.openai.bedrock export package com.openai.client.okhttp to
module com.openai.core
Relocating the class is the only fix: a deprecated forwarding class left
behind in the old package would keep the package present in both JARs and
preserve the split. The class only ever used public `OpenAIOkHttpClient`
builder methods, so its original placement was cosmetic rather than
load-bearing.
BREAKING CHANGE: `BedrockOpenAIOkHttpClient` moved from
`com.openai.client.okhttp` to `com.openai.bedrock`. Update imports to
`import com.openai.bedrock.BedrockOpenAIOkHttpClient;`.
Claude-Session: https://claude.ai/code/session_01193YsGKVxz9nkiW6qpKvFP
The published JARs carried no `Automatic-Module-Name`, so consumers on
the module path got a name derived from the JAR file name
(`openai.java.core`, `openai.java.client.okhttp`, ...). Those names are
neither namespaced nor stable across a rename, and nothing stopped them
from changing between releases.
Derive the names from the `com.openai` group ID and the packages each
artifact ships:
openai-java com.openai
openai-java-core com.openai.core
openai-java-client-okhttp com.openai.client.okhttp
openai-java-bedrock com.openai.bedrock
The mapping lives in the publish convention plugin, so it applies to
exactly the four published artifacts, and a `checkNotNull` guard fails
configuration if a newly published artifact is added without a name.
This mirrors how the root build guards the version support policy.
These names are effectively public API: once released, changing one
breaks every consumer that `requires` it.
Claude-Session: https://claude.ai/code/session_01193YsGKVxz9nkiW6qpKvFP
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5dcaf261b4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -1,9 +1,8 @@ | |||
| package com.openai.client.okhttp | |||
| package com.openai.bedrock | |||
There was a problem hiding this comment.
When users of the published 4.51.0 Bedrock artifact upgrade to the next 4.x release, moving this public class removes com.openai.client.okhttp.BedrockOpenAIOkHttpClient: existing source imports will no longer compile, and already-compiled applications can fail with NoClassDefFoundError. The version remains on the 4.x line, so this relocation should be deferred to a major release rather than shipped as an otherwise routine upgrade.
Useful? React with 👍 / 👎.
This PR adds automatic module names to all artifacts that are published. This way, users can refer to the libraries by a stable name. Currently, OpenAI module names are inferred by the jar's file name, which makes OpenAI libraries instable when used in context of the module system.
Unfortunately, there is currently a split package bug in OpenAI where both Bedrock and the OKHttp client are offered in the same package. This makes OpenAI for Java inherently incompatible with Java's module system. This requires a rename what will break API usage. I am flagging this for your decision, but this is something to clean up at some point anyways, but should likely follow a major release to flag the API breakage.