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

Update container lifetime API and add schema for lifecycleKey property #5630

Merged
merged 2 commits into from
Sep 12, 2024

Conversation

danegsta
Copy link
Member

@danegsta danegsta commented Sep 9, 2024

Description

Renames the WithContainerLifetime experimental API to WithLifetime, changes the ContainerLifetimeType enum to ContainerLifetime and renames ContainerLifetimeType.Persistent to ContainerLifetime.CreateIfNotExistsPersistOnExit as a more explicit description of the lifetime behavior.

Adds DCP schema for lifecycleKey which can be used to configure the lifetime of persistent containers (if not set, DCP will calculate a default lifecycle key value). If set, DCP will check existing persistent containers for to see if they have a matching lifecycle key. If not, the container will be destroyed and re-created based on the new AppHost config.

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 (modified newly added experimental API)
  • 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?
    • Yes
      • Link to aspire-docs issue: TBD
    • No
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 Sep 9, 2024
@danegsta danegsta enabled auto-merge (squash) September 9, 2024 21:58
@danegsta danegsta disabled auto-merge September 9, 2024 22:01
@davidfowl
Copy link
Member

Now I think we should spend some time bikeshedding about that name of this enum. If it's this explicit (and long) then we might need a helper method.

@danegsta
Copy link
Member Author

danegsta commented Sep 9, 2024

Now I think we should spend some time bikeshedding about that name of this enum. If it's this explicit (and long) then we might need a helper method.

Reusable just popped into my head as a possible option.

@danegsta
Copy link
Member Author

Now I think we should spend some time bikeshedding about that name of this enum. If it's this explicit (and long) then we might need a helper method.

Reusable just popped into my head as a possible option.

@mitchdenny @davidfowl @DamianEdwards do we want to go with the verbose CreateIfNotExistsPersistOnExit or a shorter option like Persistent/Reusable?

@davidfowl
Copy link
Member

I think I want to keep the name persistent and add a giant doc comment for what that means instead of encapsulating it in the name with a really long enum.

@danegsta
Copy link
Member Author

I reverted the name and made a pass at a more comprehensive doc comment, but I feel like the description of the behavior could still use a bit more work.

@danegsta
Copy link
Member Author

I'll merge this since we're happy with the API; any future improvements to the doc comment can be made in a follow up PR.

@danegsta danegsta merged commit 9b50510 into dotnet:main Sep 12, 2024
11 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 13, 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