-
-
Notifications
You must be signed in to change notification settings - Fork 449
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
V1: Update ElasticSearch to v1 #1088
base: develop-v1
Are you sure you want to change the base?
Conversation
ac75e96
to
cd01014
Compare
Hi @rasmus, I got some extra time so converted Elastic Search to v1, my plan is to convert from NEST to Elastic.Clients.Elasticsearch, as NEST is deprecated. It is coming in another PR :) |
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.
Pull Request Overview
This PR upgrades ElasticSearch to v1 by updating configuration, dependency injection, and logging while modernizing asynchronous patterns in read model event handling and test setups.
- Updated RELEASE_NOTES.md to announce the v1 port.
- Refactored service registrations in EventFlowOptionsExtensions to use Microsoft.Extensions.DependencyInjection.
- Converted read model event handlers to async methods and updated test configurations and logging.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
RELEASE_NOTES.md | Added notification for the v1 port of EventFlow.Elasticsearch. |
Source/EventFlow.Elasticsearch/Extensions/EventFlowOptionsExtensions.cs | Replaced legacy service registration with Microsoft.Extensions.DependencyInjection and removed obsolete methods. |
Source/EventFlow.Elasticsearch.Tests/IntegrationTests/ReadModels/ElasticsearchThingyMessageReadModel.cs | Converted Apply methods to async for better compatibility with asynchronous operations. |
Source/EventFlow.Elasticsearch.Tests/IntegrationTests/ElasticsearchReadModelStoreTests.cs | Updated DI setup and test configuration to align with the new service registration patterns. |
README.md | Changed the status indicator for EventFlow.Elasticsearch to reflect v1 compatibility. |
Source/EventFlow.Elasticsearch.Tests/IntegrationTests/ReadModels/ElasticsearchThingyReadModel.cs | Updated asynchronous event handlers with cancellation tokens. |
Source/EventFlow.Elasticsearch/ReadStores/ElasticsearchReadModelStore.cs | Updated logging to use Microsoft.Extensions.Logging and switched to async alias lookup. |
|
||
return resolver; | ||
return base.Configure(eventFlowOptions); |
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.
It appears that you are configuring services using the modified 'options', but then returning base.Configure(eventFlowOptions) which may not incorporate your new registrations. Consider returning the configured 'options' to preserve the changes made during dependency registration.
return base.Configure(eventFlowOptions); | |
return options; |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
Thanks. I'll have a look. It really awesome to get some additional help as I was hitting a wall doing it all on my own lately. Getting some support really means a lot ❤️ |
Its a awesome library you have been building! Happy to help :D |
This PR is upgrading Elastic Search from v0 to v1