Added implementation to support EventHubBufferedProducerClient #3833

WhitWaldo · 2024-04-19T12:51:06Z

Implemented my own ask from #3832 to support the EventHubBufferedProducerClient in addition to the EventHubProducerClient as part of the Aspire DI registration.

Microsoft Reviewers: Open in CodeFlow

eerhardt · 2024-04-24T17:51:40Z

@WhitWaldo - it looks like the tests are failing. Can you fix them?

WhitWaldo · 2024-04-25T12:08:09Z

@eerhardt Fixed

eerhardt · 2024-05-17T20:55:35Z

@sebastienros - can you take a look at this?

src/Components/Aspire.Azure.Messaging.EventHubs/EventHubBufferedProducerClientComponent.cs

skolima · 2024-08-09T18:57:36Z

👋 @WhitWaldo - I'm looking at the same piece of functionality missing. Would you intend to bring this up to date, or are you ok with someone else updating it?

WhitWaldo · 2024-08-10T04:50:54Z

@skolima I can bring it up to date over the weekend. It just fell off my radar!

…onger exists

WhitWaldo · 2024-08-10T23:46:11Z

This has been updated and should be ready to go now!

eerhardt · 2024-08-15T19:40:21Z

@sebastienros - can you take a look?

src/Components/Aspire.Azure.Messaging.EventHubs/AspireEventHubsExtensions.cs

radical · 2024-08-15T20:03:56Z

src/Components/Aspire.Azure.Messaging.EventHubs/AspireEventHubsExtensions.cs

+    /// <param name="configureClientBuilder">An optional method that can be used for customizing the <see cref="IAzureClientBuilder{TClient, TOptions}"/>.</param>
+    /// <remarks>Reads the configuration from "Aspire:Azure:Messaging:EventHubs:{TClient}" section, where {TClient} is the type of Event Hubs client being configured, i.e. EventProcessorClient.</remarks>
+    /// <exception cref="InvalidOperationException">Thrown when neither <see cref="AzureMessagingEventHubsSettings.ConnectionString"/> nor <see cref="AzureMessagingEventHubsSettings.FullyQualifiedNamespace"/> is provided.</exception>
+    public static void AddKeyedAzureEventHubBufferedProducerClient(


Just curious - when do we return the builder from such a method, and when do we use void?
cc @sebastienros @eerhardt

I'll leave that philosophy to others, but here I'm just mirroring the return type taken for the existing extensions (void).

radical · 2024-08-15T20:06:01Z

src/Components/Aspire.Azure.Messaging.EventHubs/EventHubBufferedProducerClientComponent.cs

+
+    protected override void BindClientOptionsToConfiguration(IAzureClientBuilder<EventHubBufferedProducerClient, EventHubBufferedProducerClientOptions> clientBuilder, IConfiguration configuration)
+    {
+#pragma warning disable IDE0200 // Remove unnecessary lambda expression - needed so the ConfigBinder Source Generator works


Just curious - what is this about? Is it a bug in the source generator that we are working around?

I simply copied the approach taken in the existing consumer client component. I don't know what it is it's referring to, but if it's necessary there, I figured it should be included in this class as well.

configuration.Bind is replaced at compiled time by a source generated version. But if you use a method group then the source generator doesn't kick in. To see the code expand the project like this:

Look for a method named Bind_EventHubBufferedProducerClientOptions and check the attribute on it that will point to the method to switch for.

WhitWaldo · 2024-08-16T13:05:22Z

All Aspire.Azure.Messaging.EventHubs.Tests are passing on my system and I believe I've spoken to each of the reviews above.

sebastienros · 2024-08-16T16:17:34Z

I think the only thing missing is updating the README to list the new client/settings. Just do a quick search for ProducerClient for the missing mentions.

sebastienros · 2024-08-16T16:31:15Z

@WhitWaldo I did the required changes in the README.

radical · 2024-08-16T16:49:28Z

/azp run

azure-pipelines · 2024-08-16T16:49:41Z

Azure Pipelines successfully started running 1 pipeline(s).

radical · 2024-08-16T18:21:27Z

Multiple tests failing like this (log):

Aspire.Azure.Messaging.EventHubs.Tests.AspireEventHubsExtensionsTests.CanAddMultipleKeyedServices(clientIndex: 2) [FAIL]
  System.InvalidOperationException : No service for type 'Azure.Messaging.EventHubs.EventProcessorClient' has been registered.
  Stack Trace:
       at Microsoft.Extensions.DependencyInjection.ServiceProvider.GetRequiredKeyedService(Type serviceType, Object serviceKey, ServiceProviderEngineScope serviceProviderEngineScope)
       at Microsoft.Extensions.DependencyInjection.ServiceProvider.GetRequiredKeyedService(Type serviceType, Object serviceKey)
       at Microsoft.Extensions.DependencyInjection.ServiceProviderKeyedServiceExtensions.GetRequiredKeyedService(IServiceProvider provider, Type serviceType, Object serviceKey)
    /_/tests/Aspire.Azure.Messaging.EventHubs.Tests/AspireEventHubsExtensionsTests.cs(259,0): at Aspire.Azure.Messaging.EventHubs.Tests.AspireEventHubsExtensionsTests.RetrieveClient(Object key, Int32 clientIndex, IHost host)
    /_/tests/Aspire.Azure.Messaging.EventHubs.Tests/AspireEventHubsExtensionsTests.cs(361,0): at Aspire.Azure.Messaging.EventHubs.Tests.AspireEventHubsExtensionsTests.CanAddMultipleKeyedServices(Int32 clientIndex)
       at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
       at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
Aspire.Azure.Messaging.EventHubs.Tests.AspireEventHubsExtensionsTests.CanAddMultipleKeyedServices(clientIndex: 3) [FAIL]
  System.InvalidOperationException : No service for type 'Azure.Messaging.EventHubs.Primitives.PartitionReceiver' has been registered.
  Stack Trace:
       at Microsoft.Extensions.DependencyInjection.ServiceProvider.GetRequiredKeyedService(Type serviceType, Object serviceKey, ServiceProviderEngineScope serviceProviderEngineScope)
       at Microsoft.Extensions.DependencyInjection.ServiceProvider.GetRequiredKeyedService(Type serviceType, Object serviceKey)
       at Microsoft.Extensions.DependencyInjection.ServiceProviderKeyedServiceExtensions.GetRequiredKeyedService(IServiceProvider provider, Type serviceType, Object serviceKey)
    /_/tests/Aspire.Azure.Messaging.EventHubs.Tests/AspireEventHubsExtensionsTests.cs(259,0): at Aspire.Azure.Messaging.EventHubs.Tests.AspireEventHubsExtensionsTests.RetrieveClient(Object key, Int32 clientIndex, IHost host)
    /_/tests/Aspire.Azure.Messaging.EventHubs.Tests/AspireEventHubsExtensionsTests.cs(361,0): at Aspire.Azure.Messaging.EventHubs.Tests.AspireEventHubsExtensionsTests.CanAddMultipleKeyedServices(Int32 clientIndex)
       at InvokeStub_AspireEventHubsExtensionsTests.CanAddMultipleKeyedServices(Object, Span`1)
       at System.Reflection.MethodBaseInvoker.InvokeWithOneArg(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)

WhitWaldo · 2024-08-16T21:43:00Z

Multiple tests failing like this (log):

Out of curiosity, is there any way to access that log without being a part of Microsoft? The tests passed locally but I saw the bot report a failure and I couldn't figure out how to access the log myself.

radical · 2024-08-16T21:52:03Z

Multiple tests failing like this (log):

Out of curiosity, is there any way to access that log without being a part of Microsoft? The tests passed locally but I saw the bot report a failure and I couldn't figure out how to access the log myself.

Are you not able to access that link? I pasted the failures here anyway - https://gist.github.com/radical/1f55987361a1a828b3ed888b081e2dbe .

…aldo/dotnet-aspire into eventhub-buffered-producer

…erClient

WhitWaldo · 2024-08-16T23:26:53Z

When I ran the tests locally before, I believe I ran them against the wrong branch. Re-resolved the comments above on the correct branch this time and fixed the unit tests + added inline tests for the new class to the mix. Now, they're all passing locally.

WhitWaldo · 2024-08-16T23:28:02Z

Multiple tests failing like this (log):

Out of curiosity, is there any way to access that log without being a part of Microsoft? The tests passed locally but I saw the bot report a failure and I couldn't figure out how to access the log myself.

Are you not able to access that link? I pasted the failures here anyway - https://gist.github.com/radical/1f55987361a1a828b3ed888b081e2dbe .

I can access the link you provided, but is there any way to access those logs from the failed check itself (e.g. assuming I don't have access to the azdo environment)?

radical · 2024-08-16T23:32:51Z

< snip />
I can access the link you provided, but is there any way to access those logs from the failed check itself (e.g. assuming I don't have access to the azdo environment)?

When the checks complete then you can see the details, and links to the failures in Build Analysis under Checks.

src/Components/Aspire.Azure.Messaging.EventHubs/EventHubBufferedProducerClientComponent.cs

radical

Other than the one comment, LGTM!

Added implementation to support EventHubBufferedProducerClient.

bf9b49b

dotnet-issue-labeler bot added the area-integrations Issues pertaining to Aspire Integrations packages label Apr 19, 2024

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 19, 2024

Removed unused line in tests

fbd6a16

Merge branch 'main' into eventhub-buffered-producer

9bc000b

eerhardt requested a review from sebastienros May 17, 2024 20:55

sebastienros reviewed May 17, 2024

View reviewed changes

src/Components/Aspire.Azure.Messaging.EventHubs/EventHubBufferedProducerClientComponent.cs Outdated Show resolved Hide resolved

Merge branch 'dotnet:main' into eventhub-buffered-producer

c793f0a

WhitWaldo requested review from radical and eerhardt as code owners August 10, 2024 23:34

WhitWaldo added 2 commits August 10, 2024 18:42

Updated to use "FullyQualifiedNamespace" property as "Namespace" no l…

ed579b6

…onger exists

Fixed XML references in comments.

8cdc7fd

radical reviewed Aug 15, 2024

View reviewed changes

Merge branch 'dotnet:main' into eventhub-buffered-producer

2e968b6

Updated README

740302a

sebastienros approved these changes Aug 16, 2024

View reviewed changes

sebastienros enabled auto-merge (squash) August 16, 2024 16:31

build-analysis bot mentioned this pull request Aug 16, 2024

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

3 tasks

WhitWaldo added 2 commits August 16, 2024 16:54

Removed unnecessary whitespace

9eb9586

Merge branch 'eventhub-buffered-producer' of https://github.com/WhitW…

d216ee0

…aldo/dotnet-aspire into eventhub-buffered-producer

auto-merge was automatically disabled August 16, 2024 21:54
Head branch was pushed to by a user without write access

WhitWaldo added 2 commits August 16, 2024 18:16

Added requested XML comment regarding the ArgumentException

cc7aff3

Fixed unit tests + added inline data tests for EventHubBufferedProduc…

696ced9

…erClient

radical reviewed Aug 17, 2024

View reviewed changes

src/Components/Aspire.Azure.Messaging.EventHubs/EventHubBufferedProducerClientComponent.cs Show resolved Hide resolved

radical approved these changes Aug 17, 2024

View reviewed changes

sebastienros merged commit 16c8d49 into dotnet:main Aug 17, 2024
11 checks passed

WhitWaldo deleted the eventhub-buffered-producer branch August 17, 2024 02:20

IEvangelist mentioned this pull request Aug 26, 2024

[Update] content - add EventHubBufferedProducerClient dotnet/docs-aspire#1558

Closed

eerhardt mentioned this pull request Aug 26, 2024

Augment EventHubs support to include EventHubBufferedProducerClient #3832

Closed

github-actions bot locked and limited conversation to collaborators Sep 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added implementation to support EventHubBufferedProducerClient #3833

Added implementation to support EventHubBufferedProducerClient #3833

WhitWaldo commented Apr 19, 2024 •

edited by dotnet-policy-service bot

Loading

eerhardt commented Apr 24, 2024

WhitWaldo commented Apr 25, 2024

eerhardt commented May 17, 2024

skolima commented Aug 9, 2024

WhitWaldo commented Aug 10, 2024

WhitWaldo commented Aug 10, 2024

eerhardt commented Aug 15, 2024

radical Aug 15, 2024

WhitWaldo Aug 16, 2024

radical Aug 15, 2024

WhitWaldo Aug 16, 2024

sebastienros Aug 16, 2024

WhitWaldo commented Aug 16, 2024

sebastienros commented Aug 16, 2024

sebastienros commented Aug 16, 2024

radical commented Aug 16, 2024

azure-pipelines bot commented Aug 16, 2024

radical commented Aug 16, 2024

WhitWaldo commented Aug 16, 2024

radical commented Aug 16, 2024

WhitWaldo commented Aug 16, 2024 •

edited

Loading

WhitWaldo commented Aug 16, 2024

radical commented Aug 16, 2024

radical left a comment

Added implementation to support EventHubBufferedProducerClient #3833

Added implementation to support EventHubBufferedProducerClient #3833

Conversation

WhitWaldo commented Apr 19, 2024 • edited by dotnet-policy-service bot Loading

Microsoft Reviewers: Open in CodeFlow

eerhardt commented Apr 24, 2024

WhitWaldo commented Apr 25, 2024

eerhardt commented May 17, 2024

skolima commented Aug 9, 2024

WhitWaldo commented Aug 10, 2024

WhitWaldo commented Aug 10, 2024

eerhardt commented Aug 15, 2024

radical Aug 15, 2024

Choose a reason for hiding this comment

WhitWaldo Aug 16, 2024

Choose a reason for hiding this comment

radical Aug 15, 2024

Choose a reason for hiding this comment

WhitWaldo Aug 16, 2024

Choose a reason for hiding this comment

sebastienros Aug 16, 2024

Choose a reason for hiding this comment

WhitWaldo commented Aug 16, 2024

sebastienros commented Aug 16, 2024

sebastienros commented Aug 16, 2024

radical commented Aug 16, 2024

azure-pipelines bot commented Aug 16, 2024

radical commented Aug 16, 2024

WhitWaldo commented Aug 16, 2024

radical commented Aug 16, 2024

WhitWaldo commented Aug 16, 2024 • edited Loading

WhitWaldo commented Aug 16, 2024

radical commented Aug 16, 2024

radical left a comment

Choose a reason for hiding this comment

WhitWaldo commented Apr 19, 2024 •

edited by dotnet-policy-service bot

Loading

WhitWaldo commented Aug 16, 2024 •

edited

Loading