Akka.Persistence - update code base to akka JVM v2.4 #1717
Conversation
Aaronontheweb
changed the title
cconstantin
changed the title
| @@ -211,39 +212,58 @@ void IDisposable.Dispose() | |||
| /// Specific table used for message persistence may be defined through configuration within | |||
| /// 'akka.persistence.journal.sql-server' scope with 'schema-name' and 'table-name' keys. | |||
| /// </summary> | |||
| public async Task WriteMessagesAsync(IEnumerable<IPersistentRepresentation> messages) | |||
| public async Task<IImmutableList<Exception>> WriteMessagesAsync(IEnumerable<AtomicWrite> messages) | |||
There was a problem hiding this comment.
I'm pretty sure IEnumerable here is sufficient. It's immutable enough in .NET space.
There was a problem hiding this comment.
Agreed. Will change.
There was a problem hiding this comment.
Actually, on second thought, IEnumerable is immutable, but the underlying implementation is not guaranteed to be, so it's possible for the creator of the IEnumerable to make changes after passing the reference back. Should we ask for a second opinion @Horusiath? :)
cconstantin
changed the title
|
Please note that the 3 failing unit tests are as a result of additional checks in TestKit. I'd recommend addressing them after merging this PR. |
|
I think it's good. Eventual changes can be implemented in separate steps. However when it comes to failing tests, they must be either fixed or skipped - without that merging this to dev will doom our CI for all other PRs waiting to be merged. |
| @@ -13,9 +14,11 @@ namespace Akka.Persistence.TestKit.Tests | |||
| public class MemoryJournalSpec : JournalSpec | |||
| { | |||
| public MemoryJournalSpec(ITestOutputHelper output) | |||
| : base(actorSystemName: "MemoryJournalSpec", output: output) | |||
| : base(typeof(MemoryJournal), "MemoryJournalSpec") | |||
There was a problem hiding this comment.
Missing output propagation to base constructor - this will cause xUnit parallel tests to not print any results.
|
@Horusiath should I open new issues for the two tests (one for journals, one for snapshot stores) that test behavior not supported yet? |
|
@Aaronontheweb I believe this is ready to be merged. Some work will be needed on updating the documentation, and to update the journals and event stores. There's also a bug in JVM akka-persistence that I'm following and will submit a PR as soon as they agree on a fix (it seems quite serious: akka/akka#19828). |
|
@cconstantin please open the issues for these tests - we don't want to forget them. |
|
@Aaronontheweb @Horusiath should I squash all commits into one before merging? |
|
@cconstantin yes please! |
|
@Aaronontheweb done |
Akka.Persistence - update code base to akka JVM v2.4
Resolves #1402.
Some of the changes:
Full list at http://doc.akka.io/docs/akka/current/project/migration-guide-2.3.x-2.4.x.html#Akka_Persistence
Only one thing left to do is to add StateChangeEvent persistence to PersistentFSM, but I believe it can be done is a subsequent release.