[Experimental] Stream Store #620
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DavidBadura
force-pushed
the
stream-store
branch
2 times, most recently
from
a8a9b71
to
8759f9a
Compare
|
Hello 👋 here is the most recent benchmark result:
This comment gets update everytime a new commit comes in! |
DavidBadura
force-pushed
the
stream-store
branch
from
8759f9a
to
767302d
Compare
DavidBadura
force-pushed
the
stream-store
branch
from
fd3b5ff
to
d2f82b8
Compare
src/Console/OutputStyle.php
Outdated
Comment on lines
112
to
119
| private function streamName(AggregateHeader|StreamHeader $header): string | ||
| { | ||
| if ($header instanceof AggregateHeader) { | ||
| return StreamNameTranslator::streamName($header->aggregateName, $header->aggregateId); | ||
| } | ||
|
|
||
| return $header->streamName; | ||
| } |
There was a problem hiding this comment.
maybe this should go into the StreamNameTranslator itself? Since this is done in another places in a different style
There was a problem hiding this comment.
Then we have an dependency aggregate->message.
Currently, the aggregate knows nothing about messages. (Except for the AggregateHeader, which will be removed in the long term)
In 4.0, StreamNameTranslator can also be removed again; it is mainly needed as a BC layer.
I want to solve this with metadata.
There was a problem hiding this comment.
You mean domain wise? I mean, the Translator could also be moved into Store, which is imho even a better place, no?
I know that this translator acts as a BC layer, if the remove the "old" store in favor of the new StreamStore. I'm fine keeping it as is right now, but if we encounter a third place, i would vote for getting this more centralized.
There was a problem hiding this comment.
How the stream name is composed is the task of the Aggregate/Repository domain. The store should not know anything about aggregate. In the future, only the repository will create this string. So, this is not a BC layer for the store, but a BC layer for the repository/aggregate which store it can work with.
I don't know if the Aggregate namespace is the right place. Maybe repository? But in my opinion, definitely not store.
DanielBadura
approved these changes
Jul 28, 2024
|
One last thing: Maybe we should add somewhere a note what to expect from a experimental feature? Like no BC compliance etc? |
|
We should generally communicate how we handle our versions. Which versions are still supported, etc. I opened an issue: #622 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces an event store that is decoupled from the aggregate root. This means that the columns
aggregateandaggregateIdhave been merged intostream. Playhead is also optional in this store.fix: #535