Skip to content
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

Add custom application model for EventHubs emulator #7005

Merged
merged 14 commits into from
Jan 10, 2025

Conversation

sebastienros
Copy link
Member

@sebastienros sebastienros commented Dec 31, 2024

Description

This PR introduces a custom model to configure the emulator and the cloud service.
Counterpart of #6737 for Event Hubs emulator.

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?
Microsoft Reviewers: Open in CodeFlow

@@ -148,8 +148,7 @@ public static IResourceBuilder<AzureEventHubsResource> RunAsEmulator(this IResou
return builder;
}

var configHostFile = Path.Combine(Path.GetTempPath(), "AspireEventHubsEmulator", Path.GetRandomFileName() + ".json");
Directory.CreateDirectory(Path.GetDirectoryName(configHostFile)!);
var configHostFile = Path.Combine(Directory.CreateTempSubdirectory("AspireEventHubsEmulator").FullName, "Config.json");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eerhardt Longer term we should have a service for doing this that gets cleaned up on exit. @karolz-ms maybe something DCP can provide since it controls the clean up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely. For example, I can imagine us having the capability for Containers where you can say "here is a blob of JSON that I need to appear as file inside the container, under this path". Sort of like Kubernetes ConfigMaps.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #7029 to track.

@eerhardt
Copy link
Member

eerhardt commented Jan 6, 2025

This will resolve #5561.

namespace Aspire.Hosting.Azure.EventHubs;

/// <summary>
/// Represents an annotation for updating the JSON content of a mounted document.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "a mounted document" mean?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Config.json file (json document) that is mounted to the container using ContainerMountType.BindMount.

src/Aspire.Hosting.Azure.EventHubs/EventHub.cs Outdated Show resolved Hide resolved
@@ -148,8 +148,7 @@ public static IResourceBuilder<AzureEventHubsResource> RunAsEmulator(this IResou
return builder;
}

var configHostFile = Path.Combine(Path.GetTempPath(), "AspireEventHubsEmulator", Path.GetRandomFileName() + ".json");
Directory.CreateDirectory(Path.GetDirectoryName(configHostFile)!);
var configHostFile = Path.Combine(Directory.CreateTempSubdirectory("AspireEventHubsEmulator").FullName, "Config.json");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #7029 to track.


writer.WriteStartObject(); // {
writer.WriteString("Name", hub); // "Name": "hub",
writer.WriteString("PartitionCount", "2"); // "PartitionCount": "2",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We used to have "2" as the default, but hub.WriteJsonObjectProperties now writes "1" by default.

Is this intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIR it was 2 because of the sample they provided. But since the schema states [1-32] I decided to use 1 when the value is not set explicitly.

/// <param name="builder">The builder for the <see cref="AzureEventHubsEmulatorResource"/>.</param>
/// <param name="path">Path to the file on the AppHost where the emulator configuration is located.</param>
/// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
public static IResourceBuilder<AzureEventHubsEmulatorResource> WithConfigurationFile(this IResourceBuilder<AzureEventHubsEmulatorResource> builder, string path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a test for this method?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was also missing in SB, so I added it. At the same time I tried to simplify the implementation. However I intentionally didn't add support for a custom file provided for config, AND configuration hooks. I would assume if you provide a file, you could have updated it yourself. Hooks are useful when the emulator is generating a file from the resource model and you can't update some section from this model.


using var builder = TestDistributedApplicationBuilder.Create().WithTestAndResourceLogging(output);
var serviceBus = builder.AddAzureServiceBus("servicebusns")
.RunAsEmulator()
.WithQueue("queue1");
.WithQueue("queue123");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this name change?

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the comments are addressed.

@sebastienros sebastienros merged commit cbf34f0 into main Jan 10, 2025
9 checks passed
@sebastienros sebastienros deleted the sebros/eventhubsmodel branch January 10, 2025 00:32
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.

4 participants