Skip to content

Conversation

@alzimmermsft
Copy link
Member

@alzimmermsft alzimmermsft commented Jun 3, 2025

Description

Updates implementation of azure-core-http-vertx to work with Vert.x's 5.0 release. Given azure-core-http-vertx was GA'd using Vert.x 4.5 it will continue using that as the dependency version, but a best effort will be made to ensure the library also works with Vert.x 5.0 (when / if possible).

Also, we cannot upgrade the package to 5.0 anyways as the baseline support changed to Java 11 rather than Java 8 and this package guaranteed Java 8 compatibility.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@alzimmermsft alzimmermsft marked this pull request as ready for review June 4, 2025 19:34
Copilot AI review requested due to automatic review settings June 4, 2025 19:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the azure-core-http-vertx implementation to ensure compatibility with Vert.x 5.0 while maintaining Java 8 support. Key changes include updating Vert.x dependency versions (from 4.5.13 to 4.5.15), refactoring several areas to use lambda-based handlers and reactive patterns (e.g. using andThen instead of legacy close callbacks), and adapting test implementations (e.g. replacing AtomicLong with BlockingQueue for progress reporting).

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
sdk/tools/azure-openrewrite-compiler-maven-plugin/pom.xml Bumps maven-core version to 3.9.9.
sdk/template/azure-template-stress/pom.xml Updates vertx-codegen version to 4.5.15 in dependency and banned dependencies.
sdk/core/azure-core/src/test/java/com/azure/core/validation/http/HttpClientTests.java Replaces AtomicLong with BlockingQueue and adds a sleep before progress assertion.
sdk/core/azure-core-http-vertx/src/test/java/com/azure/core/http/vertx/implementation/VertxRequestWriteSubscriberTests.java Refactors tests to use lambda-based write handlers instead of a custom HttpClientRequest.
sdk/core/azure-core-http-vertx/src/test/java/com/azure/core/http/vertx/implementation/MockHttpClientRequest.java Removes the custom mock class in favor of functional interfaces.
sdk/core/azure-core-http-vertx/src/test/java/com/azure/core/http/vertx/VertxHttpClientProviderTests.java, VertxHttpClientBuilderTests.java Adopts new close() API using andThen callbacks.
sdk/core/azure-core-http-vertx/src/main/java/com/azure/core/http/vertx/implementation/VertxRequestWriteSubscriber.java Refactors the subscriber to use lambda-based handlers and reactive methods.
sdk/core/azure-core-http-vertx/src/main/java/com/azure/core/http/vertx/implementation/VertxHttpResponse.java Updates netSocket close sequence with andThen.
sdk/core/azure-core-http-vertx/src/main/java/com/azure/core/http/vertx/VertxHttpClientBuilder.java, VertxHttpClient.java Adjusts builder and client implementations to use new reactive patterns and update dependency handling.
sdk/core/azure-core-http-vertx/pom.xml, eng/versioning/external_dependencies.txt, common/perf-test-core/pom.xml Updates dependency versions to reflect Vert.x 4.5.15.
Comments suppressed due to low confidence (1)

sdk/core/azure-core-http-vertx/src/main/java/com/azure/core/http/vertx/implementation/VertxRequestWriteSubscriber.java:124

  • The onComplete callback does not handle failure cases: the variable 'error' is undefined and write failures are not explicitly caught. Add an else branch to handle result failures using result.cause() and pass it to resetRequest.
resetRequest(error);

Copy link

@lmolkova lmolkova left a comment

Choose a reason for hiding this comment

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

Do we want to have multi-version matrix and test compatibility with v5?

Copy link
Member

@conniey conniey left a comment

Choose a reason for hiding this comment

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

CHANGELOG update?

@alzimmermsft
Copy link
Member Author

/check-enforcer override

@alzimmermsft
Copy link
Member Author

Failing tests are unrelated to the changes here.

@alzimmermsft alzimmermsft merged commit b46250d into Azure:main Jun 5, 2025
57 of 59 checks passed
@alzimmermsft alzimmermsft deleted the AzHttp_SupportVertx5 branch June 5, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants