Repository navigation
feat: [DevOps] Add minimal dependencies check (#388) - #1173
SoleilHoffmann wants to merge 3 commits into
Conversation
Add a 'floor-deps' Maven profile that pins consumer-facing dependencies to their minimum supported versions, plus a CI step that compiles against them so the SDK cannot accidentally rely on newer-version APIs. Documents the verified minimum versions in the release notes.
Jonas-Isr
left a comment
Jonas-Isr
left a comment
There was a problem hiding this comment.
Besides the comment on compile-test I think it looks good!
| </plugins> | ||
| </build> | ||
| <profiles> | ||
| <profile> |
There was a problem hiding this comment.
Why did you make a separate profile and GitHub step for this plugin?
Every other plugin is run as part of a local install.
Making it separate means a local install is successful and then the CI fails.
There was a problem hiding this comment.
I think it's not possible to have two different versions (the minimal floor version and latest) in one profile?
I see the problem that it doesn't fail locally but only in the CI.
I see two options from here:
- Add it into the normal lokal install --> it runs every time wich takes even longer for every local install, but would be possible.
- keep it like this, and we can run
mvn clean test -P floor-depslocally when we need to check this profile.
There was a problem hiding this comment.
If you find a way to add it to the local profile it would be great otherwise it’s not technically possible and we accept the compromise
There was a problem hiding this comment.
I did some more research and to me it sounds like it's only possible to have one dependency version per profile and per build. It would be possible to do two builds every time, with just one command (the normal maven build command would then create two builds). Same thing would be achieved by running two commands, one with floor-deps and one without.
I could also remove the CI step so we don't have the potential local/github mismatch but then we don't have a guarantee that everyone remembers to run the local floor-deps tests. (I don't like this option)
How often do we expect the test to be red anyways? If it rarely happens I think it would be fine to have that mismatch sometimes, and it's probably an easy fix, you would just have to increase the version in the profile and in the docs, not change the code.
There was a problem hiding this comment.
If it only adds 2 seconds to the build then merge it, if it’s way more then leave it out of the local build
There was a problem hiding this comment.
I tested it, it was about 25s.
Added Jonas' suggestions test instead of test-compile, and clean test Co-authored-by: Jonas-Isr <jonas.israel@sap.com>
…388) Running the floor check as 'clean test' (not just test-compile) revealed two floors that compiled but failed at runtime: - SAP Cloud SDK 5.28.0 calls HttpClient5 5.4+ APIs (TlsSocketStrategy, RequestConfig.setProtocolUpgradeEnabled). Raise HttpComponents floor to what Cloud SDK 5.28 brings in (httpcore5 5.4.2, httpclient5 5.6). - Jackson 3 / Spring AI 2.0 (via victools jsonschema-generator 5) needs jackson-annotations >= 2.22 (JsonApplyView). jackson-annotations versions independently from databind/core, so split it into its own floor property (databind/core stays 2.18.0, annotations 2.22).
Add a
floor-depsMaven profile that pins consumer-facing dependencies to their minimum supported versions, plus a CI step that compiles against them so the SDK cannot accidentally rely on newer-version APIs.Documents the verified minimum versions in the release notes.
Context
AI/ai-sdk-java-backlog#388.
There is a matching v1 PR
Proof that the check is green with the correct versions
Proof that the check is red with the wrong versions
Slack Thread
Feature scope:
floor-depsmaven profileDefinition of Done
Aligned changes with the JavaScript SDK