Adding Hosting Extensions for Akka.Persistence.MongoDB #331
Conversation
src/Akka.Persistence.MongoDb.Hosting.Tests/Akka.Persistence.MongoDb.Hosting.Tests.csproj
Outdated
Show resolved
Hide resolved
| <Import Project="..\common.props" /> | ||
|
|
||
| <PropertyGroup> | ||
| <TargetFrameworks>$(NetFrameworkTestVersion);$(NetTestVersion);$(NetCoreTestVersion)</TargetFrameworks> |
There was a problem hiding this comment.
$(NetFrameworkTestVersion) - I did not have the Netframework developer kit installed on my machine so the tests have not been run against framework. Given the changes I don't believe it will be an issue but if required I will get them installed. They were all run against NetTestVersion and NetCoreTestVersion
There was a problem hiding this comment.
No worries - looks like things are passing on CI/CD so far. We can probably drop .NET Framework for testing MongoDb in the near future anyway.
src/Akka.Persistence.MongoDb.Hosting.Tests/Akka.Persistence.MongoDb.Hosting.Tests.csproj
Outdated
Show resolved
Hide resolved
|
|
||
| namespace Akka.Persistence.MongoDb.Hosting.Tests | ||
| { | ||
| public class MongoDbJournalOptionsSpec |
There was a problem hiding this comment.
These tests are basically clones of the ones here https://github.com/akkadotnet/Akka.Persistence.PostgreSql/blob/dev/src/Akka.Persistence.PostgreSql.Tests/Hosting/PostgreSqlOptionsSpec.cs
I have split them up and cleaned up parts but if there are additional cases needed I am happy to add them.
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Akka.Persistence.Hosting" Version="$(AkkaVersion)" /> |
There was a problem hiding this comment.
From the other projects it looked like targeting $(AkkaVersion) was the correct approach.
| /// </exception> | ||
| public static AkkaConfigurationBuilder WithMongoDbPersistence( | ||
| this AkkaConfigurationBuilder builder, | ||
| string connectionString, |
There was a problem hiding this comment.
In this method I have only added the connectionString as a parameter that is custom on top of the default. For this plugin specifically are there any others that you would suggest adding?
| @@ -0,0 +1,41 @@ | |||
| # Akka.Persistence.MongoDb.Hosting | |||
There was a problem hiding this comment.
Some input if this is sufficient for a readme would be appreciated. Is there anything additional that would be useful to be covered.
| options.Serializer.Should().Be("hyperion"); | ||
|
|
||
| // Dispose called here as project not using latest language features. | ||
| stream.Dispose(); |
There was a problem hiding this comment.
Added this to both tests as the using declarations are not currently available in the language features for this project and it caused build failures. Do you have a preferred way of handling this?
And to get the build to run again
|
It looks like the test failures are due to timeouts. This time it was the windows build and previously it was the linux one. Is there a better way to handle the build/test runs? |
src/Akka.Persistence.MongoDb.Hosting.Tests/Akka.Persistence.MongoDb.Hosting.Tests.csproj
Outdated
Show resolved
Hide resolved
| <Import Project="..\common.props" /> | ||
|
|
||
| <PropertyGroup> | ||
| <TargetFrameworks>$(NetFrameworkTestVersion);$(NetTestVersion);$(NetCoreTestVersion)</TargetFrameworks> |
There was a problem hiding this comment.
No worries - looks like things are passing on CI/CD so far. We can probably drop .NET Framework for testing MongoDb in the near future anyway.
src/Akka.Persistence.MongoDb.Hosting.Tests/Akka.Persistence.MongoDb.Hosting.Tests.csproj
Outdated
Show resolved
Hide resolved
| /// <summary> | ||
| /// Transaction | ||
| /// </summary> | ||
| public bool UseWriteTransaction { get; set; } |
There was a problem hiding this comment.
What's the default value for this?
There was a problem hiding this comment.
Explicit default has been set - Confirmed it matches reference conf.
| /// <summary> | ||
| /// Transaction | ||
| /// </summary> | ||
| public bool UseWriteTransaction { get; set; } |
There was a problem hiding this comment.
Explicit default has been set - Confirmed it matches reference conf from snapshot-store section.
Fixes #329
Changes
These changes add the extension methods to support wiring up Akka.Persistence.MongoDB with Akka.Hosting.
These changes are heavily influenced by the way the hosting extensions were created in the Postgres repo (https://github.com/akkadotnet/Akka.Persistence.PostgreSql/). If there are any additional test cases that could be added please let me know.
I have a different version of these changes running in an application and I have not come across any initial issues. The application does not use persistence extensively so there might be some edge cases I am not aware of that need to be handled.
Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):