Event Hubs: Update EventProcessorClient component to pull BlobClient from DI #3293
Conversation
…mponents; update processor for DI retrieval of blob client
dotnet-issue-labeler
bot
added
the
area-integrations
dotnet-policy-service
bot
added
the
community-contribution
|
fyi @eerhardt |
| @@ -114,7 +116,7 @@ private sealed class TableServiceComponent : AzureComponent<AzureDataTablesSetti | |||
| return !string.IsNullOrEmpty(connectionString) ? new TableServiceClient(connectionString, options) : | |||
| cred is not null ? new TableServiceClient(settings.ServiceUri, cred, options) : | |||
| new TableServiceClient(settings.ServiceUri, options); | |||
| }, requiresCredential: false); | |||
There was a problem hiding this comment.
What is the impact of changing this? I see that it's always true in AddClient methods
There was a problem hiding this comment.
We shouldn't be changing this. We need to keep the current behavior.
There was a problem hiding this comment.
I fixed most changes back to the way they were. I missed the EventHubs ones, but I need to step away. Can you take care of that? I can take care of it when I get back too.
| settings.ConsumerGroup ?? EventHubConsumerClient.DefaultConsumerGroupName, settings.Namespace, | ||
| settings.EventHubName, cred, options); | ||
|
|
||
| return processor; | ||
|
|
||
| }, requiresCredential: false); |
There was a problem hiding this comment.
There is no available method that provides options, cred, provider and requiresCredential. The ultimate private call has these but there is no public combination that allows to access it. Ideally on AddClient since this is the one we wanted. I assume the expectation was that if you provide a factory with TokenCredential you'd want requiresCredential: true but here we don't always.
There was a problem hiding this comment.
I was able to successfully use this using a connection string. So I'm not sure we need to specify requiresCredential: false. Since this is a new component, we shouldn't be breaking anyone. But let's keep the other components doing what they are doing now. This one can use AddClient.
@tg-msft - do you know what this requiresCredential bool does/means exactly? Why can't I specify requiresCredential: false when I use the overloads with IServiceProvider?
There was a problem hiding this comment.
Yeah, this is the problem I had -- there was no public overload available that allowed specifying it when you want the service provider. I couldn't immediately see what the point of this flag even was, so I figured it was vestigial as everything seemed to work regardless.
There was a problem hiding this comment.
requiresCredential is a niche feature for services that allow anonymous auth (i.e., reads on a Storage account used for hosting static websites). I believe it only controls whether or not we throw an exception when resolving a client if there was no credential provided. https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/extensions/Microsoft.Extensions.Azure/src/Internal/ClientRegistration.cs#L46
Remove the need to specify a BlobClientServiceKey, and use an unkeyed client if none is provided.
src/Components/Aspire.Azure.Messaging.EventHubs/AzureMessagingEventHubsSettings.cs
Outdated
Show resolved
Hide resolved
…from DI (dotnet#3293) * update sample EH project launchsettings; add blob component for processor * update azurecomponent to expose serviceprovider; update associated components; update processor for DI retrieval of blob client * throw if blobclient cannot be retrieved from DI (this is a breaking change from P5) * fix tests * PR feedback * More PR feedback * Rename BlobClientConnectionName to BlobClientServiceKey. Remove the need to specify a BlobClientServiceKey, and use an unkeyed client if none is provided. * fix tests * fixup formatting --------- Co-authored-by: Eric Erhardt <[email protected]> Co-authored-by: Sebastien Ros <[email protected]>
The BlobServiceClient is first looked for as a keyed service using
settings.BlobConnectionName-- theserviceKeyis always the same as theconnectionNamefor keyed services. If this is not found, a default service is looked for. If this is not found, anInvalidOperationExceptionis thrown with a helpful message. This is a breaking change from Preview 5.Closes #3254
Microsoft Reviewers: Open in CodeFlow