-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable Rust based AMQP by default #6362
Merged
LarryOsterman
merged 55 commits into
Azure:feature/rust_amqp
from
LarryOsterman:larryo/enable-rust-amqp
Jan 29, 2025
Merged
Enable Rust based AMQP by default #6362
LarryOsterman
merged 55 commits into
Azure:feature/rust_amqp
from
LarryOsterman:larryo/enable-rust-amqp
Jan 29, 2025
Conversation
This file contains 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
API change check APIView has identified API level changes in this PR and created following API reviews. |
danieljurek
reviewed
Jan 22, 2025
LarryOsterman
requested review from
weshaggard,
benbp,
antkmsft,
gearama and
RickWinter
as code owners
January 27, 2025 23:59
LarryOsterman
force-pushed
the
feature/rust_amqp
branch
from
January 28, 2025 15:01
689e9fa
to
07c4983
Compare
…uild configurationj to reflect that
LarryOsterman
force-pushed
the
larryo/enable-rust-amqp
branch
from
January 28, 2025 15:01
b248dd0
to
548029b
Compare
Ping :). |
RickWinter
reviewed
Jan 28, 2025
RickWinter
reviewed
Jan 29, 2025
RickWinter
reviewed
Jan 29, 2025
RickWinter
approved these changes
Jan 29, 2025
LarryOsterman
added a commit
that referenced
this pull request
Jan 30, 2025
* AMQP tests now pass; Integrate TestAmqpBroker with CI pipeline * Updated changelogs to reflect API changes made during AMQP updates * Replace uAMQP with Rust AMQP as the default AMQP transport; Updated build configurationj to reflect that * Test fixes * PR feedback; test fixes
LarryOsterman
added a commit
that referenced
this pull request
Feb 4, 2025
* AMQP tests now pass; Integrate TestAmqpBroker with CI pipeline * Updated changelogs to reflect API changes made during AMQP updates * Replace uAMQP with Rust AMQP as the default AMQP transport; Updated build configurationj to reflect that * Test fixes * PR feedback; test fixes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Enable Rust based AMQP stack.
Azure::Nullable<AmqpValue>
becauseAmqpValue
is inherently nullable, soNullable<AmqpValue>
is redundantFetchContent_Populate
(which is deprecated) toFetchContent_MakeAvailable
- this silences an annoying warning in the CI pipeline.NOTE:
That this PR lays the foundation for enabling Rust by default, but it does not complete the work. There is additional work needed to fix mac related issues and some internal-to-fe2o3 issues that have been found during the CI pipeline runs. But this PR is already rather large so this is effectively a checkpoint of a work-in-progress.
GitHub CoPilot Summary:
This pull request introduces several changes to the build configuration and codebase to support the integration of Rust-based APIs in the C++ SDK, while also addressing compatibility issues and making various improvements. The most important changes include enabling and temporarily disabling Rust in the build, updating the CMake configuration for UWP builds, and modifying the AMQP library integration.
Build Configuration Updates:
CMakeLists.txt
: Added options to enable Rust-based APIs and disable AMQP and OpenTelemetry functionalities. Temporarily disabled Rust in the build.CMakePresets.json
: Renamed presets to reflect the disabling of Rust AMQP implementation and added new presets for Linux clang-18 builds. [1] [2] [3] [4] [5]eng/pipelines/templates/stages/platform-matrix.json
: Removed the argument to disable OpenTelemetry for UWP release builds.AMQP Library Integration:
sdk/core/CMakeLists.txt
: Modified the condition to include the AMQP library based on theDISABLE_AMQP
flag instead ofBUILD_WINDOWS_UWP
.sdk/core/azure-core-amqp/CMakeLists.txt
: Added logic to enable or disable the Rust-based AMQP stack and configure Rust for x86 builds. [1] [2] [3]Code Changes:
sdk/core/azure-core-amqp/inc/azure/core/amqp/models/amqp_properties.hpp
: RemovedAzure::Nullable
fromAmqpValue
types inMessageProperties
. [1] [2] [3]sdk/core/azure-core-amqp/src/impl/rust_amqp/amqp/message_receiver.cpp
: Added a check to throw an exception if the context is canceled.sdk/core/azure-core-amqp/src/impl/rust_amqp/amqp/message_sender.cpp
: Reordered initialization of member variables in the constructor.Test Setup and Cleanup:
sdk/core/azure-core-amqp/Test-Setup.ps1
: Added a script to clone, build, and start the Test Amqp Broker.sdk/core/azure-core-amqp/Test-Cleanup.ps1
: Added a script to stop the Test Amqp Broker job.