-
Notifications
You must be signed in to change notification settings - Fork 521
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
add elasticsearch hosting #4430
Conversation
@dotnet-policy-service agree |
playground/Elasticseach/Elasticseach.AppHost/Properties/launchSettings.json
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Elasticsearch/ElasticsearchBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Elasticsearch/ElasticsearchBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
tests/Aspire.Hosting.Tests/Elasticsearch/AddElasticsearchTests.cs
Outdated
Show resolved
Hide resolved
tests/Aspire.Hosting.Tests/Elasticsearch/AddElasticsearchTests.cs
Outdated
Show resolved
Hide resolved
tests/Aspire.Hosting.Tests/Elasticsearch/AddElasticsearchTests.cs
Outdated
Show resolved
Hide resolved
@Alirexaa could you please create a GitHub issue over on https://github.com/dotnet/docs-aspire with a article request and link it back to this PR. We want to make sure that we are bringing up the docs along with the feature changes like this one to help make your contribution more discoverable. |
So I think on the The component side needs more work (as @davidfowl alluded to). We will probably want an Overall, it is looking pretty good. |
src/Aspire.Hosting.Elasticsearch/ElasticsearchContainerImageTags.cs
Outdated
Show resolved
Hide resolved
This The httpclient retry log messages show:
The container log dumped at the end shows:
Another thing to note is that this test ran to the timeout for all of the tests - 15mins. So maybe we need to bump that. But that is if the container failure is irrelevant. |
It seems container stopped. |
@@ -35,6 +35,7 @@ public IntegrationServicesTests(ITestOutputHelper testOutput, IntegrationService | |||
[InlineData(TestResourceNames.sqlserver)] | |||
[InlineData(TestResourceNames.efsqlserver)] | |||
[InlineData(TestResourceNames.milvus)] | |||
[InlineData(TestResourceNames.elasticsearch)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we have "FunctionalTests" in ElasticsearchFunctionalTests, I don't think we need to add this to the EndToEnd tests.
Using the new separate Hosting tests pattern, along with functional tests there, means we don't need to have every resource/component pair in the EndToEnd tests. Instead we can just use some "core" resources and components in the EndToEnd, and the other resources can be tested in their own assembly.
…csearch libraries This allows us to ship these libraries as preview even when the rest of the Aspire libraries ship stable. Will remove and ship stable once the libraries have had time to get feedback.
Fixed the merge conflicts. |
@eerhardt Tests failing with: |
# Conflicts: # tests/Shared/RepoTesting/Directory.Packages.Helix.props # tests/testproject/TestProject.AppHost/TestProject.AppHost.csproj # tests/testproject/TestProject.IntegrationServiceA/TestProject.IntegrationServiceA.csproj
…tests that cover the same functionality.
…sticsearch-hosing
I've pushed the image to our mirror. This should be fixed now. |
FYI - I've removed the EndToEnd additions as they are no longer necessary now that we have functional tests for Elasticsearch. |
@Alirexaa thank you once again for your contribution to .NET Aspire. This PR took a while to get merged and we appreciate you hanging in there whilst we go through multiple review iterations. Just so you know we plan on shipping this in .NET Aspire 8.1 as a preview package to allow us to gather some broader usage feedback with a few of marking the package and API as stable in .NET Aspire 8.2 when it rolls around. We are in the process of writing the blog post for .NET Aspire 8.2, are you happy for us to give you a shout out and link to your GitHub profile in that post? |
I will be testing this (preview) package. @Alirexaa thanks for your work. I could now completely implement Aspire in my current project, while the only missing part was Elastic search. |
Glad to working here. Thanks to you and team that gave feedback to PR.
Yes sure. |
Glad to hear. Thanks for you feedback. |
close: Add Elasticsearch component #4418
Microsoft Reviewers: Open in CodeFlow