Update Netty and OkHttp dependencies and resolve test failures - #3066
Conversation
There was a problem hiding this comment.
⚠️ Attention Required: Lockfile Detected
This pull request contains modifications to one or more *.lockfile files. Please confirm that you have run update_dependency.sh to push new dependencies to the private repo.
Someone with Admin role must manually dismiss this review before merging.
This commit updates several dependencies in dependencies.gradle and resolves the resulting test failures: - Netty: Updated strict upper bound from <4.2.0 to <5.0.0 (resolved to 4.2.14.Final). - OkHttp: Removed strict upper bound of <5.0.0 and moved to dynamic versions section (resolved to 5.3.2). - Cleaned up dependencies.gradle by moving strict dependencies (secretmanager, common-protos, gax) out of the dynamic versions section. - Updated all gradle.lockfile files. Resulting test fixes: - Netty: Replaced NioEventLoopGroup with DefaultEventLoopGroup in NettyExtension and ProbingStepTest. In Netty 4.2+, LocalServerChannel no longer supports registration with NIO-backed event loops, throwing an IllegalArgumentException. - OkHttp: Refactored ServiceMonitoringClient to safely handle empty responses using Optional.orElse(). In OkHttp 5.x, a 204 No Content response returns a non-null ResponseBody with an empty string, causing an assertion failure where MosApiException was incorrectly bypassed.
There was a problem hiding this comment.
⚠️ Attention Required: Lockfile Detected
This pull request contains modifications to one or more *.lockfile files. Please confirm that you have run update_dependency.sh to push new dependencies to the private repo.
Someone with Admin role must manually dismiss this review before merging.
gbrodman
left a comment
There was a problem hiding this comment.
did you run the update_dependency.sh script?
@gbrodman reviewed 11 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on CydeWeys).
CydeWeys
left a comment
There was a problem hiding this comment.
Yes. In fact, this entire PR is basically human-generated except for the commit description, which I had the AI write.
@CydeWeys made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on CydeWeys).
CydeWeys
left a comment
There was a problem hiding this comment.
(Also I don't know how you'd get all those .lockfile changes without having run that script.)
@CydeWeys made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on CydeWeys).
gbrodman
left a comment
There was a problem hiding this comment.
have you never just updated the lockfiles?
./gradlew anyTask --write-locks regenerates the lockfiles for the task you give it. The update_dependency.sh script runs ./gradlew build --write-locks as part of the process.
@gbrodman made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on CydeWeys).
CydeWeys
left a comment
There was a problem hiding this comment.
I can't say that I have ever only updated the lockfiles, no. The only time I'm ever updating the lockfiles, I'm also going to want it to upload the new versions of the dependencies to GCS (if it succeeds).
@CydeWeys made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on CydeWeys).
This commit updates several dependencies in dependencies.gradle and resolves the resulting test failures:
Resulting test fixes:
This change is