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

Move resource creation in ResourceReadyEvent event #7211

Merged
merged 2 commits into from
Jan 23, 2025
Merged

Conversation

sebastienros
Copy link
Member

@sebastienros sebastienros commented Jan 22, 2025

Description

I am testing the new ResourceReadyEvent. However there is a limitation in this case, I would still prefer the previous solution. Resource creation (db, containers) needs to happen when the emulator is "ready". But dependents should not use cosmosdb until the resources are created.

The previous solution was relying on a custom healthcheck to create and assess the db/containers are created and/or ready. This new solution is hitting a deadlock, that is the ResourceReadyEvent is triggered once the resource si healthy, but if the healthcheck ensure the db/containers are available then it can't work since ResourceReadyEvent is doing it. So right now no db/containers are passed in the healthcheck, so it will only ensure the service is available before ResourceReadyEvent can create the db/containers.

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?

@eerhardt
Copy link
Member

This looks more like what I expect. But maybe thinking about it more .... not considering the resource to be "healthy" until the database is created might be the right thing to do....

@davidfowl
Copy link
Member

We can do that as a follow up to this #7163

@sebastienros sebastienros marked this pull request as ready for review January 23, 2025 00:46
@davidfowl davidfowl merged commit 174f923 into main Jan 23, 2025
9 checks passed
@davidfowl davidfowl deleted the sebros/cosmosready branch January 23, 2025 02:43
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.

3 participants