-
-
Notifications
You must be signed in to change notification settings - Fork 307
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
feat: Add Azure EventHubs module #1183
feat: Add Azure EventHubs module #1183
Conversation
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
private readonly AzuriteContainer _azuriteContainer; | ||
|
||
private EventHubsContainer _eventHubsContainer; | ||
|
||
private readonly INetwork _network = new NetworkBuilder().WithName(NetworkName).Build(); | ||
|
||
private const string NetworkName = "eh-network"; | ||
private const string AzuriteNetworkAlias = "azurite"; | ||
|
||
private const string EventHubName = "testeventhub"; | ||
private const string EventHubConsumerGroupName = "testconsumergroup"; | ||
|
||
private EventHubsContainerTest() | ||
{ | ||
_azuriteContainer = new AzuriteBuilder() | ||
.WithNetwork(_network) | ||
.WithNetworkAliases(AzuriteNetworkAlias) | ||
.Build(); | ||
} |
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.
This is an idea still but due to EventHubs emulator will not work without Azurite then we can provide two ways to run EventHub Container implementation.
- Embed Azurite, so, users don't worry about it.
- Allow external Azurite in case users need a specific configuration
/cc @kiview WDYT?
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.
Does a pattern exist already ?
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.
recently, service bus was added with the same pattern. See https://github.com/testcontainers/testcontainers-dotnet/blob/develop/src/Testcontainers.ServiceBus/ServiceBusBuilder.cs#L114
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.
I think it makes sense to align with the pattern Service Bus is using and then follow up with something that allows us to use an existing Azurite instance. Please also see this issue: #1323. This is something we need to figure out and support in general.
Also, please add documentation for the module. |
Hi @WakaToa / @eddumelendez / @kiview , I am from Azure Event Hubs Product team and was looking at this PR. I do realize that there were some plausible solutions offered above to get this integration setup. Could you please let us know if we could extend any help from our side to integrate Event Hubs emulator with testcontainers framework? We'll be happy to provide any product related assistance here. |
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.
Thanks for the PR. There are a few things we need to address before we can merge the PR.
{ | ||
Validate(); | ||
|
||
var waitStrategy = Wait.ForUnixContainer().UntilMessageIsLogged("Emulator Service is Successfully Up!"); |
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 wait strategy should be moved to Init()
.
@@ -0,0 +1,53 @@ | |||
namespace Testcontainers.EventHubs.Configuration |
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.
Please change all to file scoped namespaces.
public EventHubsBuilder WithConfigurationBuilder(ConfigurationBuilder configurationBuilder) | ||
{ | ||
var configBytes = Encoding.UTF8.GetBytes(configurationBuilder.Build()); | ||
|
||
return Merge(DockerResourceConfiguration, new EventHubsConfiguration(configurationBuilder: configurationBuilder)) | ||
.WithResourceMapping(configBytes, "Eventhubs_Emulator/ConfigFiles/Config.json"); | ||
} |
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.
Does this builder cover all Event Hubs configuration? Are we responsible for it? Is there a default configuration? The EventHubsBuilder
will fail if the user does not call WithConfigurationBuilder(ConfigurationBuilder)
explicitly. Until now, no other module requires an additional explicit call to a builder method (expect license agreements). _ = new FooBuilder.Build()
always returns a working configuration. This is something I favor.
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.
With configuration builder is needed, because we don't have a default eventhub assigned to the default namespace. We can eventually introduce something, but I don't advise to do so as usually we want to have something custom. For an example, we have azure function:
public static class ProcessSalesData
{
[FunctionName("ProcessSalesData")]
public static void Run(
[EventHubTrigger("SalesDataHub", Connection = "EventHubConnectionAppSetting", ConsumerGroup = "RealTimeAnalytics")] string[] events,
ILogger log)
{
foreach (var eventData in events)
{
log.LogInformation($"C# Event Hub trigger function processed an event: {eventData}");
}
}
}
Our eventhub is SalesDataHub
and usually in integration tests we won't enforce to change it to the ones that is created by the testcontainer.eventhubs. We'd rather to replicate solution that we have in our infra with custom event hub names and just automatically subscribe to this when new message came up.
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.
On a second thought we can introduce eh1
by default. Always we will be able to expand that even further, but for most of the time maybe that will be enought (for most scenarios).
{ | ||
return base.Init() | ||
.WithImage(EventHubsImage) | ||
.WithEnvironment("ACCEPT_EULA", "Y") |
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.
Please do not hide license agreements. Right now, we use the following pattern.
/// <summary> | ||
/// Gets the configuration builder | ||
/// </summary> | ||
public ConfigurationBuilder ConfigurationBuilder { get; } |
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.
It looks like we do not use the ConfigurationBuilder
sometime later. In that case, we do not need to store it in the EventHubsConfiguration
.
@@ -0,0 +1,127 @@ | |||
using DotNet.Testcontainers; |
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.
Please use the global usings file.
|
||
private EventHubsContainer _eventHubsContainer; | ||
|
||
private readonly INetwork _network = new NetworkBuilder().WithName(NetworkName).Build(); |
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.
Please do not use static network names. This may clash with already existing resources and may prevent tests from running in parallel. See our best practices.
private readonly AzuriteContainer _azuriteContainer; | ||
|
||
private EventHubsContainer _eventHubsContainer; | ||
|
||
private readonly INetwork _network = new NetworkBuilder().WithName(NetworkName).Build(); | ||
|
||
private const string NetworkName = "eh-network"; | ||
private const string AzuriteNetworkAlias = "azurite"; | ||
|
||
private const string EventHubName = "testeventhub"; | ||
private const string EventHubConsumerGroupName = "testconsumergroup"; | ||
|
||
private EventHubsContainerTest() | ||
{ | ||
_azuriteContainer = new AzuriteBuilder() | ||
.WithNetwork(_network) | ||
.WithNetworkAliases(AzuriteNetworkAlias) | ||
.Build(); | ||
} |
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.
I think it makes sense to align with the pattern Service Bus is using and then follow up with something that allows us to use an existing Azurite instance. Please also see this issue: #1323. This is something we need to figure out and support in general.
@@ -0,0 +1,22 @@ | |||
<Project Sdk="Microsoft.NET.Sdk"> | |||
<PropertyGroup> | |||
<TargetFrameworks>net8.0</TargetFrameworks> |
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.
<TargetFrameworks>net8.0</TargetFrameworks> | |
<TargetFrameworks>net9.0</TargetFrameworks> |
<ItemGroup> | ||
<PackageVersion Update="Azure.Messaging.EventHubs" Version="5.11.3" /> | ||
</ItemGroup> |
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.
<ItemGroup> | |
<PackageVersion Update="Azure.Messaging.EventHubs" Version="5.11.3" /> | |
</ItemGroup> |
Already included above.
What does this PR do?
Microsoft released an emulator for EventHubs yesterday, see Azure/azure-service-bus#223 (comment)
This PR included a module for the emulator.
Info
You need to provide an Azurite (emulator) instance/endpoint to start the EventHub emulator. This can be done using the
EventHubsConfiguration
.You need to specify the following configuration file when starting the emulator.
It will be dynamically mapped and i have built a floating builder so that the user can set the configuration as easily as possible.
The namespace "emulatorNs1" is mandatory and it is the only one currently supported by the emulator.