Skip to content

Feature/solr integration #802

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Sep 4, 2025
Merged

Feature/solr integration #802

merged 16 commits into from
Sep 4, 2025

Conversation

@Chicoo
Copy link
Contributor

@Chicoo Chicoo commented Aug 14, 2025

**Closes #792 **

Adds a hosting integration for the Solr container. It implements a health check to see if the created core is available and exposes the full URL to the core as a connection string.

PR Checklist

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • New integration
    • Docs are written
    • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

Other information

Chicoo added 5 commits August 10, 2025 10:39
@Chicoo
Added Solr Hosting to community toolkit
1047d84
@Chicoo
Added Solr Hosting to community toolkit
2374f07
@Chicoo
feat: Complete Solr hosting integration with core name support and he…
6ed7f4c 
…alth checks

- Add SolrResource with CoreName property and connection string support
- Implement SolrBuilderExtensions with AddSolr method for flexible configuration
- Add SolrHealthCheck for core-specific health monitoring using admin/ping endpoint
- Support custom ports and core names with automatic core pre-creation
- Provide connection strings with format: http://{host}:{port}/solr/{coreName}
- Add comprehensive unit tests covering all configuration scenarios
- Add integration tests with Docker container validation
- Update example AppHost with default and custom Solr configurations
- Add complete README with installation instructions and Apache Solr link
- Clean up using statements and optimize imports
- All 11 tests passing with full Docker-based validation
@Chicoo
refactor: Simplify Solr health check implementation
0006590 
- Replace custom SolrHealthCheck class with built-in AddUrlGroup method
- Remove SolrHealthCheck.cs file for cleaner codebase
- Use direct URI construction for core-specific health endpoints
- Maintain same functionality with less complexity and better maintainability
- All 11 tests still passing with simplified approach
@Chicoo
Merge branch 'CommunityToolkit:main' into feature/solr-integration
8599f30
Copilot AI review requested due to automatic review settings August 14, 2025 17:59
Copilot AI reviewed Aug 14, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new hosting integration for Apache Solr container resources in .NET Aspire Community Toolkit. It provides a comprehensive implementation with container configuration, health checks, and connection string management for Solr cores.

Key Changes

  • Adds a complete Solr hosting integration with configurable port and core name parameters
  • Implements health check functionality to verify Solr core availability via admin ping endpoint
  • Provides connection string expression that includes the full URL path to the specific Solr core

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/CommunityToolkit.Aspire.Hosting.Solr/SolrBuilderExtensions.cs Main extension method for adding Solr resources with container configuration and health checks
src/CommunityToolkit.Aspire.Hosting.Solr/SolrResource.cs Resource class defining Solr container with connection string and endpoint properties
src/CommunityToolkit.Aspire.Hosting.Solr/SolrContainerImageTags.cs Container image configuration using Solr 9.7 from docker.io
tests/CommunityToolkit.Aspire.Hosting.Solr.Tests/SolrResourceTests.cs Unit tests for resource creation and configuration
tests/CommunityToolkit.Aspire.Hosting.Solr.Tests/AppHostTests.cs Integration tests verifying container startup and health checks
examples/solr/Program.cs Example application demonstrating Solr resource usage
Various project and solution files Project structure and dependency configuration

@Chicoo
Copy link
Contributor Author

Chicoo commented Aug 14, 2025

@dotnet-policy-service agree

@Chicoo
Update src/CommunityToolkit.Aspire.Hosting.Solr/SolrBuilderExtensions.cs
f37111e 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
davidfowl
davidfowl reviewed Aug 18, 2025
View reviewed changes
aaronpowell
aaronpowell requested changes Aug 21, 2025
View reviewed changes
Copy link
Member

@aaronpowell aaronpowell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to update tests.yaml so that the new test project is detected - if you check out the failing check "Verify Test List" it'll have a diff of the change needed.

src/CommunityToolkit.Aspire.Hosting.Solr/SolrHealthCheck.cs Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's no need for a custom health check, let's remove this file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The integration adds a Solr Core to the container and the healthcheck is needed to check if the core is available. Otherwise it cannot be used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, but there's no code in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn, missed that. I moved the health check to SolrBuilderExtensions as that felt cleaner, but forgot to remove the HealthCheck file.
File is removed now.

Chicoo and others added 7 commits August 24, 2025 15:58
aaronpowell
aaronpowell requested changes Aug 27, 2025
View reviewed changes
src/CommunityToolkit.Aspire.Hosting.Solr/SolrHealthCheck.cs Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, but there's no code in this file

aaronpowell
aaronpowell approved these changes Sep 1, 2025
View reviewed changes
Copy link
Member

@aaronpowell aaronpowell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I'll merge it shortly and we'll targe the CT 9.8 release for it to be released.

@aaronpowell aaronpowell merged commit 6148295 into CommunityToolkit:main Sep 4, 2025
102 checks passed
This was referenced Sep 26, 2025
Open
Closed
Open
Open
Closed
This was referenced Sep 26, 2025
Open
Closed
Closed
Closed
Merged
Merged
This was referenced Oct 7, 2025
Closed
Closed
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidfowl davidfowl davidfowl left review comments

Copilot code review Copilot Copilot left review comments

@aaronpowell aaronpowell aaronpowell approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add Apache Solr Integration

3 participants

@Chicoo @davidfowl @aaronpowell