Add polyglot exports for minio#1190
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.sh | bash -s -- 1190Or
iex "& { $(irm https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.ps1) } 1190" |
There was a problem hiding this comment.
Pull request overview
This PR adds polyglot (AspireExport) coverage for the MinIO hosting integration and introduces a TypeScript ValidationAppHost to validate the generated exports.
Changes:
- Annotates MinIO resource + builder extension APIs with
AspireExportfor polyglot tooling. - Adds a TypeScript ValidationAppHost (TS config + npm setup + apphost script) for MinIO.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/CommunityToolkit.Aspire.Hosting.Minio/MinioContainerResource.cs | Exports the MinIO resource and exposes properties for polyglot access. |
| src/CommunityToolkit.Aspire.Hosting.Minio/MinioBuilderExtensions.cs | Exports the MinIO builder/extension methods for polyglot consumption. |
| playground/polyglot/TypeScript/CommunityToolkit.Aspire.Hosting.Minio/ValidationAppHost/tsconfig.json | TypeScript compiler configuration for the ValidationAppHost. |
| playground/polyglot/TypeScript/CommunityToolkit.Aspire.Hosting.Minio/ValidationAppHost/package.json | npm package definition for running/building the ValidationAppHost. |
| playground/polyglot/TypeScript/CommunityToolkit.Aspire.Hosting.Minio/ValidationAppHost/package-lock.json | Lockfile for deterministic installs. |
| playground/polyglot/TypeScript/CommunityToolkit.Aspire.Hosting.Minio/ValidationAppHost/apphost.ts | Validation apphost exercising exported MinIO APIs and exported properties. |
| playground/polyglot/TypeScript/CommunityToolkit.Aspire.Hosting.Minio/ValidationAppHost/apphost.run.json | Local run profile settings for the ValidationAppHost. |
| playground/polyglot/TypeScript/CommunityToolkit.Aspire.Hosting.Minio/ValidationAppHost/.aspire/settings.json | Aspire polyglot settings pointing to the TS apphost and package under test. |
Files not reviewed (1)
- playground/polyglot/TypeScript/CommunityToolkit.Aspire.Hosting.Minio/ValidationAppHost/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
src/CommunityToolkit.Aspire.Hosting.Minio/MinioBuilderExtensions.cs:102
WithPassword(...)doesn’t validate thepasswordargument. Since it is dereferenced viapassword.Resource, passing null will throw aNullReferenceException. AddArgumentNullException.ThrowIfNull(password);(matchingWithUserName) to fail fast with a clear exception.
public static IResourceBuilder<MinioContainerResource> WithPassword(this IResourceBuilder<MinioContainerResource> builder, IResourceBuilder<ParameterResource> password)
{
ArgumentNullException.ThrowIfNull(builder);
builder.Resource.SetPassword(password.Resource);
return builder;
You can also share your feedback on Copilot code review. Take the survey.
| #pragma warning disable ASPIREATS001 // AspireExport is experimental | ||
|
|
| #pragma warning disable ASPIREATS001 // AspireExport is experimental | ||
|
|
||
| namespace Aspire.Hosting.ApplicationModel; |
|
I'm glad to see this, but main MiniO integration currently undergoes deprecation process due to the main storage being deprecated. I guess this working integration wouldn't hurt, but need second opinion on this. Also adding test coverage can be beneficial |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
a90b62a to
639ab6a
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Minimum allowed line rate is |
Squash merge of #1190 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds AspireExport coverage and a matching TypeScript validation apphost for minio.