-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
RabbitMQ client version upgrade #1367
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
base: master
Are you sure you want to change the base?
RabbitMQ client version upgrade #1367
Conversation
…efore setting value
…kflow - Added `should_execute_json_workflow_with_complex_input_type` test to verify handling of complex input properties. - Ensured `AssignTask` step correctly sets `Assignee` information in `FlowData`. - Included JSON workflow definition with complex input property. - Verified `Assignee` and `UnitInfo` properties are correctly set in the test.
…n methods to use async/await
@nurlanmikayilov it looks like there are some additional changes in addition to the RabbitMQ upgrade. Was that intentional? |
@danielgerlag, mainly yes — the additional changes are minor, such as setting some collections to their default values. Let me know if you’d prefer I split them into a separate PR. |
There was a problem hiding this 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 upgrades the RabbitMQ.Client NuGet package from version 6.8.1 to 7.1.2 to resolve version conflicts and MethodNotFoundException errors. The upgrade required updating the RabbitMQ provider code to use the new async API methods introduced in version 7.x.
- Updated RabbitMQ.Client package reference to version 7.1.2
- Migrated RabbitMQ provider code to use async methods (CreateChannelAsync, BasicPublishAsync, etc.)
- Added support for complex input properties in workflow definitions including list and array types
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/providers/WorkflowCore.QueueProviders.RabbitMQ/WorkflowCore.QueueProviders.RabbitMQ.csproj | Updates RabbitMQ.Client package reference to version 7.1.2 |
src/providers/WorkflowCore.QueueProviders.RabbitMQ/Services/RabbitMQProvider.cs | Migrates to async RabbitMQ API methods and updates connection handling |
src/providers/WorkflowCore.QueueProviders.RabbitMQ/ServiceCollectionExtensions.cs | Updates connection factory to use async methods |
src/WorkflowCore.DSL/Services/DefinitionLoader.cs | Adds support for list input processing and improves object input handling |
test/WorkflowCore.TestAssets/* | Adds test data classes and workflow definitions for complex input property testing |
test/WorkflowCore.IntegrationTests/Scenarios/StoredJsonScenario.cs | Adds integration tests for complex input property scenarios |
Comments suppressed due to low confidence (2)
_connection.CloseAsync(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CloseAsync() method call is not awaited. This should be 'await _connection.CloseAsync();' or the method should be synchronous if async is not intended in Dispose().
_connection.CloseAsync(); | |
} | |
_connection.CloseAsync().GetAwaiter().GetResult(); | |
_connection = null; | |
} | |
public async ValueTask DisposeAsync() | |
{ | |
if (_connection == null) return; | |
if (_connection.IsOpen) | |
await _connection.CloseAsync(); | |
_connection = null; | |
} |
Copilot uses AI. Check for mistakes.
options.Services.AddSingleton<RabbitMqConnectionFactory>( | ||
sp => (provider, name) => | ||
{ | ||
var connection = rabbitMqConnectionFactory(provider, name).GetAwaiter().GetResult(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using GetAwaiter().GetResult() can lead to deadlocks in certain contexts. Consider using async/await pattern or ConfigureAwait(false) to avoid potential deadlock scenarios.
var connection = rabbitMqConnectionFactory(provider, name).GetAwaiter().GetResult(); | |
var connection = rabbitMqConnectionFactory(provider, name).ConfigureAwait(false).GetAwaiter().GetResult(); |
Copilot uses AI. Check for mistakes.
"unitType": 1 | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are trailing spaces after the closing brace. This should be cleaned up for consistency.
} | |
} |
Copilot uses AI. Check for mistakes.
Description of Change
Upgraded the RabbitMQ.Client NuGet package to version 7.1.2 and made necessary adjustments to ensure compatibility with the updated API.
Implementation Details
This update was necessary to fix a MethodNotFoundException caused by version conflicts—other dependencies in the project were referencing a newer version of RabbitMQ.Client, which led to runtime errors. Aligning the versions resolved the issue.
Tests
No new tests were added, as the changes were limited to dependency upgrades and minor compatibility adjustments. Existing tests continue to pass successfully.
Breaking Changes
This change is not expected to break existing functionality. However, consumers relying on older versions of RabbitMQ.Client should verify compatibility.
Additional Context
The upgrade ensures consistency across project dependencies and prevents runtime issues due to version mismatches.
⸻
If you have any feedback or would like to see additional refinements, feel free to let me know—I’m happy to make further improvements if needed.