-
Notifications
You must be signed in to change notification settings - Fork 39
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
[Feature] Dapr - Azure extensions + Redis configuration #349
Comments
I'm unfamiliar with how Dapr works, but if I'm understanding, this is a proposal to add this API: var daprState = builder.AddDaprStateStore("daprState")
.WithReference(redisState); And that gives you a resource you can provide to another resource, such as the API project in the above sample, to have an opinionated pre-configured Dapr integration. Is that right? |
Feels like these should go with the dapr integration |
I guess there are two parts to the proposal the first part (CommunityToolkit.Aspire.Dapr.AzureExtensions) adds a set of functions that can be used to configure the dapr resources directly from the apphost rather than requiring supplementary yaml files for the dapr config The second part is a specific implementation as above for redis (CommunityToolkit.Aspire.Dapr.AzureRedis) which adds the
i.e already exists as part of Aspire.Hosting.Dapr but requires additional configuration based on yaml files. This uses withreference to configure the dapr state based on the
above. Note the current implementation has no bearing on local development, only on publishing Two readme files might help |
Couple of things I considered here:
|
Cool, that all makes sense. The question on whether this should be in the Aspire repo (likely as new packages due to the Azure dependency) or the Community Toolkit is something that we'd need the Aspire team to weigh in on. |
@davidfowl / @aaronpowell - Is there anything I can / need to do to progress this? I realise the majority of people are probably still on xmas break - I was hoping to get this at least mostly done before I head back to work next week |
I think, unfortunately, the people that I need input on regarding whether this should be in the Community Toolkit or the Aspire Dapr package are on leave still (@davidfowl, @mitchdenny, etc.). |
I think we have 3 options:
I've spoken to @philliphoff and the dapr team about 3, but it feels like 2 feels like reasonable approach. The code above is exactly the vision for dapr component aspire integration. On top of that it should work in a first-class way with dapr on ACA (for the supported components there). We should have enough primitives exposed to make this possible to support without changes to the core itself. The last complication is that we have Aspire.Hosting.Dapr today. Would we move that as is or do we deprecate this package and move to CommunityToolkit.Aspire.Hosting.Dapr. I think this package would be better off in the hands of strong community leadership that is using it and can help it evolve quickly. There's LOTS of potential to simplify the workflow here and it's just not a priority for the core team. |
My vote is on #2 |
If migration is the path forward - I'm happy to do the work provided as part of my what I've already done. At this point I feel like I've spent enough time staring at the code to understand it. 😬 |
I would vote for (2) as well, if (3) isn't possible.
I think this is the key. As you mention above "and it's just not a priority for the core team." makes it hard to evolve in the dotnet/aspire repo, as the core team would need to review/approve PRs and triage issues.
Is the rename 100% necessary? We could move the package as-is to this repo, if we wanted. |
Yeah I can just agree with @davidfowl. Was thinking the same when I opened in the main repo the idea/draft PR of a better integration of dapr. (dotnet/aspire#6965) @FullStackChef are you in the dapr discord? Maybe we can connect. Because I see we both have similar ideas and potentially implement the same things. |
@aaronpowell How do you feel about that? Does it muddy things? I think it does a bit but I wouldn't die on this hill OK executive decision, we move the dapr integration to the community toolkit. @philliphoff wrote the integration and is one of the dapr sdk maintainers so I would like to him to chime in at the very least on the vision 😄 |
@paule96 I am in the dapr discord. collaboration sounds good to me. @davidfowl @eerhardt
|
Reasons off the top of my head:
Like I said, I won't die on a hill for this one though. |
Yeah, I'm not convinced 100% either way is the "right way". Just having a discussion and trying to get my head around the thinking. If we were to have started this project in the CommunityToolkit repo, we would have used the But, if we were to follow option (3) above and move the dapr Aspire integration to dapr .NET client team, we definitely wouldn't use the This is one of the reasons I dislike having ownership in library/package names. Ownership of a library can change over time, and having to rename the package just because the ownership changed isn't ideal. When we moved the AWS Aspire integration to a new repo, we didn't need to rename the package. aws/integrations-on-dotnet-aspire-for-aws#1. But maybe the thing that makes this situation special is that all the packages coming out of the CommunityToolkit are explicitly named |
@davidfowl I was asking the opposite - sorry poor semantics, My question is what is the value of leaving the name as is? It seems like a pretty trivial amount of effort to change? @eerhardt I don't disagree on the naming convention and ownership - except to say that I feel like consistency is better than perfection - the rest of the community toolkit is named as such it seems strange to name something differently |
|
I think that’s a feature, letting people know that this package is deprecated which the package manager is good at now. It’s minimally breaking and we can shift the docs in a clean way over to the toolkit |
Wakes up, ready for another easy day to start the new year. What the notifications! 🤣 I'd be comfortable for the Community Toolkit to take over the ownership of the Dapr integration, with the expectation that:
For deprecation of the existing package, it could take a dependency on the Community Toolkit package and mark the existing types/methods as |
I like it. |
Nah I'm out 🤣 (JK) Just to be clear. Prior to Aspire I had no knowledge of Dapr - I looked at it because I figured if there was an aspire integration it must be useful. I have found that to be correct and both the aspire + dapr communities to be super helpful. I've got enough coding experience and (probably more importantly) I'm stubborn enough to figure things out. I've tried to replicate the approaches in other aspire integrations and I'm super keen to contribute but I don't claim to be an expert by any stretch and my experience is fully based in azure. That said if others like @paule96 are keen to contribute and there are people with knowledge to provide some direction then I'll happily do the work |
@davidfowl & @aaronpowell Could I please get some feedback on #368 I've migrated Aspire.Hosting.Dapr however attempting to migrate unit tests is a rabbit warren that runs deep into internals of the Aspire.Hosting libraries It would be good if I can get some guidance on how to approach this without rewriting aspire 😅 |
Why not do the lift and shift and the azure dapr extensions in different PRs? That'll make it easier to review. There's lots of new code to look at in the azure dapr extensions. |
Why not indeed: A) You changed the title of my issue and now it doesn't match the summary - this is my attempt at payback 🤣 Jokes aside, makes a lot of sense to separate these, I think we should :
This will be the least amount of re-work for me, and the azure / redis integration can be reviewed etc separately to the lift and shift |
Changes the title back. Let’s open a new issue with the migration. |
Hello all! I'm one of the Dapr .NET maintainers. Someone shared this thread with me yesterday and I thought I'd share my own thoughts here instead of just the Dapr Discord channel. It's great to see that we've all found a common ground regarding the ongoing development and maintenance of the Aspire and Dapr integration. This addresses the concerns raised at the end of 2024:
Once this has achieved some degree of maturity in this repository, there's also nothing that prevents us from revisiting this conversation about lifting and integrating a feature-complete tool out from CommunityToolkit and into the Dapr project for long-term maintenance. Thank you to everyone involved for your collaboration and efforts in making this possible and please, don't hesitate to ping me on an issue here (@WhitWaldo), on the Dapr Discord channel (Whit), or send me a PM directly on Discord (username: Xaniff) if you need any assistance. |
I will chime in too as a (former?) Dapr .NET maintainer and original developer of the Dapr & Aspire integration. I'm glad there is renewed interest in using Dapr with Aspire and that others seem willing to pick up that torch. As much as I love Dapr (and Aspire), my current work no longer affords me the opportunity to contribute in an official capacity and my list of "side projects" is already a mile long (and decidedly kid-focused at the moment--3D printing ideas, anyone?). I'm more than willing to chat about the current implementation, possible future directions, etc., but, as much as I'd like to, it's unlikely that I can contribute myself (for the time being). I'm just happy if it finds a place and people willing to keep it moving forward. |
.NET Aspire issue link
No response
Overview
Usage example
Additional context
I have this on a branch ready to go into a PR
Help us help you
Yes, I'd like to be assigned to work on this item
The text was updated successfully, but these errors were encountered: