Skip to content

Commit

Permalink
Continue removing sync API
Browse files Browse the repository at this point in the history
Part of #1472

* Remove the last sync I/O method from `IFrameHandler`

Update action versions to address Node.js warnings

* Remove use of `EnsureCompleted`

* Handle connection start using async/await

* Remove `EnsureCompleted` task extensions.

* Remove `BasicConsumeRpcContinuation`

* Update action versions to address Node.js warnings

* Remove non-async RPC continuation classes

* Remove unused classes

* Address some TODOs

* Set a `TimeoutException` when an async RPC continuation times out
  • Loading branch information
lukebakken committed Feb 26, 2024
1 parent ec129ba commit 752923c
Show file tree
Hide file tree
Showing 19 changed files with 53 additions and 721 deletions.
34 changes: 17 additions & 17 deletions .github/workflows/build-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
with:
submodules: true
- name: Cache NuGet packages
uses: actions/cache@v3
uses: actions/cache@v4
with:
path: |
~/.nuget/packages
Expand All @@ -32,7 +32,7 @@ jobs:
- name: Unit Tests
run: dotnet test "${{ github.workspace }}\projects\Test\Unit\Unit.csproj" --no-restore --no-build --logger 'console;verbosity=detailed'
- name: Upload Build (Debug)
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4
with:
name: rabbitmq-dotnet-client-build-win32
path: |
Expand All @@ -52,14 +52,14 @@ jobs:
with:
submodules: true
- name: Cache installers
uses: actions/cache@v3
uses: actions/cache@v4
with:
# Note: the cache path is relative to the workspace directory
# https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#using-the-cache-action
path: ~/installers
key: ${{ runner.os }}-v0-${{ hashFiles('.ci/versions.json') }}
- name: Download Build (Debug)
uses: actions/download-artifact@v3
uses: actions/download-artifact@v4
with:
name: rabbitmq-dotnet-client-build-win32
path: projects
Expand All @@ -81,7 +81,7 @@ jobs:
"${{ github.workspace }}\projects\Test\Integration\Integration.csproj" --no-restore --no-build --logger 'console;verbosity=detailed'
- name: Maybe upload RabbitMQ logs
if: failure()
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4
with:
name: rabbitmq-logs-integration-win32
path: ~/AppData/Roaming/RabbitMQ/log/
Expand All @@ -97,14 +97,14 @@ jobs:
with:
submodules: true
- name: Cache installers
uses: actions/cache@v3
uses: actions/cache@v4
with:
# Note: the cache path is relative to the workspace directory
# https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#using-the-cache-action
path: ~/installers
key: ${{ runner.os }}-v0-${{ hashFiles('.ci/versions.json') }}
- name: Download Build (Debug)
uses: actions/download-artifact@v3
uses: actions/download-artifact@v4
with:
name: rabbitmq-dotnet-client-build-win32
path: projects
Expand All @@ -115,7 +115,7 @@ jobs:
run: dotnet test --environment "RABBITMQ_RABBITMQCTL_PATH=${{ steps.install-start-rabbitmq.outputs.path }}" "${{ github.workspace }}\projects\Test\SequentialIntegration\SequentialIntegration.csproj" --no-restore --no-build --logger 'console;verbosity=detailed'
- name: Maybe upload RabbitMQ logs
if: failure()
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4
with:
name: rabbitmq-logs-sequential-integration-win32
path: ~/AppData/Roaming/RabbitMQ/log/
Expand All @@ -128,11 +128,11 @@ jobs:
with:
submodules: true
- name: Setup .NET
uses: actions/setup-dotnet@v3
uses: actions/setup-dotnet@v4
with:
dotnet-version: 6.x
- name: Cache NuGet packages
uses: actions/cache@v3
uses: actions/cache@v4
with:
path: |
~/.nuget/packages
Expand All @@ -149,7 +149,7 @@ jobs:
- name: Unit Tests
run: dotnet test "${{ github.workspace }}/projects/Test/Unit/Unit.csproj" --no-restore --no-build --logger 'console;verbosity=detailed'
- name: Upload Build (Debug)
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4
with:
name: rabbitmq-dotnet-client-build-ubuntu
path: |
Expand All @@ -166,11 +166,11 @@ jobs:
with:
submodules: true
- name: Setup .NET
uses: actions/setup-dotnet@v3
uses: actions/setup-dotnet@v4
with:
dotnet-version: 6.x
- name: Download Build (Debug)
uses: actions/download-artifact@v3
uses: actions/download-artifact@v4
with:
name: rabbitmq-dotnet-client-build-ubuntu
path: projects
Expand All @@ -188,7 +188,7 @@ jobs:
"${{ github.workspace }}/projects/Test/Integration/Integration.csproj" --no-restore --no-build --logger 'console;verbosity=detailed'
- name: Maybe upload RabbitMQ logs
if: failure()
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4
with:
name: rabbitmq-logs-integration-ubuntu
path: ${{ github.workspace }}/.ci/ubuntu/log/
Expand All @@ -201,11 +201,11 @@ jobs:
with:
submodules: true
- name: Setup .NET
uses: actions/setup-dotnet@v3
uses: actions/setup-dotnet@v4
with:
dotnet-version: 6.x
- name: Download Build (Debug)
uses: actions/download-artifact@v3
uses: actions/download-artifact@v4
with:
name: rabbitmq-dotnet-client-build-ubuntu
path: projects
Expand All @@ -219,7 +219,7 @@ jobs:
"${{ github.workspace }}/projects/Test/SequentialIntegration/SequentialIntegration.csproj" --no-restore --no-build --logger 'console;verbosity=detailed'
- name: Maybe upload RabbitMQ logs
if: failure()
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4
with:
name: rabbitmq-logs-sequential-integration-ubuntu
path: ${{ github.workspace }}/.ci/ubuntu/log/
4 changes: 2 additions & 2 deletions .github/workflows/oauth2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ jobs:
- uses: actions/checkout@v4
with:
submodules: true
- uses: actions/setup-dotnet@v3
- uses: actions/setup-dotnet@v4
with:
dotnet-version: 6.x
- uses: actions/cache@v3
- uses: actions/cache@v4
with:
path: |
~/.nuget/packages
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish-nuget.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
- uses: actions/checkout@v4
with:
submodules: true
- uses: actions/cache@v3
- uses: actions/cache@v4
with:
path: |
~/.nuget/packages
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public void GlobalSetup()

var cf = new ConnectionFactory { ConsumerDispatchConcurrency = 2 };
// TODO / NOTE: https://github.com/dotnet/BenchmarkDotNet/issues/1738
_connection = cf.CreateConnectionAsync().EnsureCompleted();
_connection = EnsureCompleted(cf.CreateConnectionAsync());
}

[GlobalCleanup]
Expand All @@ -37,5 +37,7 @@ public Task Publish_Hello_World()
{
return Networking_BasicDeliver_Commons.Publish_Hello_World(_connection, messageCount, _body);
}

private static T EnsureCompleted<T>(Task<T> task) => task.GetAwaiter().GetResult();
}
}
28 changes: 0 additions & 28 deletions projects/RabbitMQ.Client/client/TaskExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,34 +203,6 @@ static async ValueTask DoTimeoutAfter(ValueTask valueTask, TimeSpan timeout)
#endif
}

/*
* https://devblogs.microsoft.com/dotnet/configureawait-faq/
* I'm using GetAwaiter().GetResult(). Do I need to use ConfigureAwait(false)?
* Answer: No
*/
public static void EnsureCompleted(this Task task)
{
task.GetAwaiter().GetResult();
}

public static T EnsureCompleted<T>(this Task<T> task)
{
return task.GetAwaiter().GetResult();
}

public static T EnsureCompleted<T>(this ValueTask<T> task)
{
return task.GetAwaiter().GetResult();
}

public static void EnsureCompleted(this ValueTask task)
{
if (false == task.IsCompletedSuccessfully)
{
task.GetAwaiter().GetResult();
}
}

#if NETSTANDARD
// https://github.com/dotnet/runtime/issues/23878
// https://github.com/dotnet/runtime/issues/23878#issuecomment-1398958645
Expand Down
15 changes: 5 additions & 10 deletions projects/RabbitMQ.Client/client/framing/Channel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -108,23 +108,19 @@ protected override Task<bool> DispatchCommandAsync(IncomingCommand cmd, Cancella
}
case ProtocolCommandId.BasicCancelOk:
{
bool result = HandleBasicCancelOk(in cmd);
return Task.FromResult(result);
return Task.FromResult(false);
}
case ProtocolCommandId.BasicConsumeOk:
{
bool result = HandleBasicConsumeOk(in cmd);
return Task.FromResult(result);
return Task.FromResult(false);
}
case ProtocolCommandId.BasicGetEmpty:
{
bool result = HandleBasicGetEmpty(in cmd);
return Task.FromResult(result);
return Task.FromResult(false);
}
case ProtocolCommandId.BasicGetOk:
{
bool result = HandleBasicGetOk(in cmd);
return Task.FromResult(result);
return Task.FromResult(false);
}
case ProtocolCommandId.BasicNack:
{
Expand Down Expand Up @@ -165,8 +161,7 @@ protected override Task<bool> DispatchCommandAsync(IncomingCommand cmd, Cancella
}
case ProtocolCommandId.ConnectionStart:
{
HandleConnectionStart(in cmd);
return Task.FromResult(true);
return HandleConnectionStartAsync(cmd, cancellationToken);
}
case ProtocolCommandId.ConnectionTune:
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public AsyncRpcContinuation(TimeSpan continuationTimeout)
// TODO LRB rabbitmq/rabbitmq-dotnet-client#1347
// Cancellation was successful, does this mean we should set a TimeoutException
// in the same manner as BlockingCell?
tcs.SetException(new TimeoutException("TODO LRB rabbitmq/rabbitmq-dotnet-client#1347"));
}
}, _tcs);
#else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,6 @@ internal AutorecoveringConnection(ConnectionConfig config, IEndpointResolver end
_endpoints = endpoints;
}

internal IConnection Open()
{
IFrameHandler fh = _endpoints.SelectOneAsync(_config.FrameHandlerFactoryAsync, CancellationToken.None).EnsureCompleted();
CreateInnerConnection(fh);
_innerConnection.Open();
return this;
}

internal async ValueTask<IConnection> OpenAsync(CancellationToken cancellationToken)
{
IFrameHandler fh = await _endpoints.SelectOneAsync(_config.FrameHandlerFactoryAsync, cancellationToken)
Expand Down
Loading

0 comments on commit 752923c

Please sign in to comment.