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

Remove RedisContainerResource and containerize RedisResource. #2073

Merged
merged 3 commits into from
Feb 5, 2024

Conversation

mitchdenny
Copy link
Member

@mitchdenny mitchdenny commented Feb 5, 2024

Related: #1741

Rather than one big change that changes all resources at once, I am going do one resource at a time to allow for easier merges with other people working across the codebase.

What this PR does is remove the AddRedisContainer(...) method and instead:

  • Makes RedisResource derive from ContainerResource.
  • Removes IRedisResource as it is no longer required for generic constraints.
  • Enhances WithAnnotation(...) to support some AddReplace and AddAppend semantics.
  • Adds a PublishAsContainer extension which subsitutes the manifest writer ot make sure a container.v0 resource is produced.
Microsoft Reviewers: Open in CodeFlow

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Feb 5, 2024
@mitchdenny mitchdenny self-assigned this Feb 5, 2024
@mitchdenny mitchdenny added this to the preview 4 (Mar) milestone Feb 5, 2024
@mitchdenny mitchdenny merged commit 1b94656 into main Feb 5, 2024
8 checks passed
@mitchdenny mitchdenny deleted the mitchdenny/reds-resource-consolidation branch February 5, 2024 06:56
// this code to accomodate it.
if (behavior != ResourceAnnotationMutationBehavior.Append && behavior != ResourceAnnotationMutationBehavior.Replace)
{
throw new ArgumentOutOfRangeException(nameof(behavior), behavior, "ResourceAnnotationMutationBehavior must be either AddAppend or AddReplace.");
Copy link
Member

Choose a reason for hiding this comment

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

Does Add prefix need to be dropped here? If so, then maybe use nameof(..) so it is in sync.

radical pushed a commit to radical/aspire that referenced this pull request Feb 6, 2024
…#2073)

* Remove RedisContainerResource and containerize RedisResource.

* Fix up tests.

* Test doesn't need PublishAsContainer()..
@github-actions github-actions bot locked and limited conversation to collaborators Apr 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants