Skip to content

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

nurlanmikayilov
Copy link

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.

…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.
@danielgerlag
Copy link
Owner

@nurlanmikayilov it looks like there are some additional changes in addition to the RabbitMQ upgrade. Was that intentional?

@nurlanmikayilov
Copy link
Author

@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.

@danielgerlag danielgerlag requested a review from Copilot August 8, 2025 16:05
Copy link

@Copilot 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 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)

Comment on lines +81 to 83
_connection.CloseAsync();
}

Copy link
Preview

Copilot AI Aug 8, 2025

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().

Suggested change
_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();
Copy link
Preview

Copilot AI Aug 8, 2025

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.

Suggested change
var connection = rabbitMqConnectionFactory(provider, name).GetAwaiter().GetResult();
var connection = rabbitMqConnectionFactory(provider, name).ConfigureAwait(false).GetAwaiter().GetResult();

Copilot uses AI. Check for mistakes.

"unitType": 1
}
}
}
Copy link
Preview

Copilot AI Aug 8, 2025

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.

Suggested change
}
}

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants