Add SurrealDB hosting/client integration#251
Add SurrealDB hosting/client integration#251Odonno wants to merge 5 commits intoCommunityToolkit:mainfrom
Conversation
|
@Odonno thanks for the contribution. For runnig CI Actions, Please fix the confilict with the main brach. |
aaronpowell
left a comment
aaronpowell
left a comment
There was a problem hiding this comment.
A good start and I can see how it'll fit into the Toolkit overall. As @Alirexaa mentioned, there's some merge conflicts that will block running a CI job on this.
I've left some comments/changes throughout the PR too.
| /examples/surrealdb/_ @Odonno | ||
| /src/CommunityToolkit.Aspire.Hosting.SurrealDb/_ @Odonno | ||
| /tests/CommunityToolkit.Aspire.Hosting.SurrealDb.Tests/_ @Odonno | ||
| /src/CommunityToolkit.Aspire.SurrealDb/_ @Odonno | ||
| /tests/CommunityToolkit.Aspire.SurrealDb.Tests/_ @Odonno No newline at end of file |
There was a problem hiding this comment.
For the CODEOWNERS to be valid you would need to join the maintainer group - is that something you're interested in (I'm guessing so since you put yourself down as the owner 😉)
There was a problem hiding this comment.
That should be good now. Can you reopen the PR @aaronpowell ?
There was a problem hiding this comment.
I'm unable to reopen the PR, looks like the branch has had a force push done since it was closed so the PR isn't able to reopen
...ire.Hosting.SurrealDb.ApiService/CommunityToolkit.Aspire.Hosting.SurrealDb.ApiService.csproj
Show resolved
Hide resolved
...it.Aspire.Hosting.SurrealDb.AppHost/CommunityToolkit.Aspire.Hosting.SurrealDb.AppHost.csproj
Show resolved
Hide resolved
| namespace CommunityToolkit.Aspire.Hosting.SurrealDb.Tests; | ||
|
|
||
| [RequiresDocker] | ||
| public class SurrealDbFunctionalTests(ITestOutputHelper testOutputHelper) |
There was a problem hiding this comment.
I'm cautious on the maintainability of these tests, as we've had some problems in the past with this style in addition to the ones that use the examples app host project.
Once we're able to run CI on them, let's evaluate properly, but also consider what these tests are testing that the functionality the Community Toolkit provides vs what is .NET Aspire. For example, if the volume persistence fails, is that a Toolkit bug or Aspire bug? Assuming all our tests cover setting the right annotations isn't the Toolkit doing the right stuff?
There was a problem hiding this comment.
I can't really tell. But at least it can confirm that the database integration works as expected for those scenarii.
|
|
||
| namespace CommunityToolkit.Aspire.Hosting.SurrealDb.Tests; | ||
|
|
||
| public class SurrealDbPublicApiTests |
There was a problem hiding this comment.
Let's get some tests in here that assert all the right annotations are setup across the container, the mounts, args, etc.
There was a problem hiding this comment.
Do you have an example? I suppose Verify would be useful for this kind of tests?
src/CommunityToolkit.Aspire.Hosting.SurrealDb/SurrealDbBuilderExtensions.cs
Show resolved
Hide resolved
src/CommunityToolkit.Aspire.Hosting.SurrealDb/SurrealDbBuilderExtensions.cs
Show resolved
Hide resolved
src/CommunityToolkit.Aspire.Hosting.SurrealDb/SurrealDbContainerImageTags.cs
Show resolved
Hide resolved
3 participants
Closes #<ISSUE_NUMBER>
Project Additions and Updates
Dependency Updates
SurrealDb.Netas a dependency of the SurrealDB client integrationBogusto generate fake data, to be used for testing purposesSurrealDb.MinimalApis.Extensionsto provide extension methods for Minimal APIs. Used to create API endpoints for theexampleAPI service project with the minimum of code.Code Ownership
CODEOWNERSfile to include new SurrealDB projects and assigned ownership to @OdonnoPR Checklist
Other information
Unable to run tests that RequiresDocker as of now. For some reasons, the containers are never created. Maybe this is the reason. I am using Rancher Desktop. For info, those tests worked in the previous PR I mentioned.