-
Notifications
You must be signed in to change notification settings - Fork 24
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
Revision / Expected Version should be an internal concern? #18
Comments
Hi Onam, The So if you want a new stream, pass in 0. If you got a concurrency exception from that you know the stream exists. When appending to an existing stream, you know if another process has got in first if you get a concurrency exception assuming you've supplied the correct value (you can get this value when reading the stream). Do you think some examples in the docs would help here, or do you think a change in API would make sense? Just wary of adding additional operations when the existing one covers quite a few use cases. It's very similar to how Event Store's client API works (though I believe they have some constants you can pass in, but it's still an int). |
We're using SES wrapped in a simple event sourcing library too, containing three classes. It exposes the concept of an EventStream (wrapping an EventStore and simplifying this revision management question) and also an AggregateBase class that can repopulate an aggregate from an EventStream and append events to the stream, managing the revision. The only dependency of this tiny 3-class library is SES. There was a lot of discussion over approaches for the Aggregate - the base class is not ideal, but when we looked at it from the consuming component's point of view, this seemed to result in the simplest possible API to handle aggregates and event streams. Is it stretching the responsibility boundary to add EventStream and/or Aggregate concepts into SES? At the moment it is an Event Store. This would add support for Event Sourcing. A small argument in favour might be if the majority of use cases for SES involve the same approach of projecting a stream via an Aggregate. SES would then support those use cases with a simpler API (not replacing the existing pure event-store API). Would need to revisit the existing Simple Event Store packages (non-ASOS, different project, same name) to check we're not going down the same road. The ASOS approach is simpler, less feature-rich (in a good way) and that point of difference between the two might be important. |
The current idea is to not add an aggregate abstraction into SES. The reason for this is that event sourcing can be done with or without DDD. For example you could have a simple transaction script that handles this (guessing this is where you'd use your EventStream abstraction). |
True, thanks. |
@mikegore1000 I guess the documentation could provide a little more detail about this and its usage. Perhaps an example would help? The current example is very basic so perhaps some guidelines (best practice) to managing the revision number I would personally find very useful. Also, the revision number I get the impression this needs to be persisted somewhere? Reason I ask this is because if I were to read the stream from somewhere other than zero, I can't rely on the revision number being the number of events processed. This is why I guess it doesn't feel correct for the client to manage this and arguably where would you store this.. on the event or somewhere else entirely? |
Shameless plug: I've put a .NET Standard library together called TinyAggregate which might help with this. It doesn't have any dependencies on SES but they do play nice together. As the name suggests, TinyAggregate is focussed on the DDD side of things. Within the YMMV but it works well with my projects that use SES. NuGet: TinyAggregate |
Noticed that the
expected revision
needs to be supplied in when persisting a stream and thought, shouldn't this be an internal concern?When consuming this for the first time I was a little confused what this was and arguably should it be the consumers responsibility to manage this?
The text was updated successfully, but these errors were encountered: