SAFE-Stack / SAFE-ConfPlanner

Working sample of a SAFE-Stack project that uses CQRS/Event-Sourcing and the Elm Architecture
The Unlicense
152 stars 15 forks source link

Fix postgres implementation #73

Closed CallumVass closed 6 months ago

CallumVass commented 4 years ago

Hi,

I've been doing some research on EventSourcing recently and discovered this project, however upon cloning it I noticed the Postgres implementation of the event storage was commented out, when I included it in the fsproj file I then noticed it was broken in several places. I've attempted to fix them and got the project building now.

Feedback welcome :)

rommsen commented 4 years ago

Hey Callum,

thanks for the PR. The question is why we want to activate the Postgres Support. The whole "EventSourced" project in here is taken from my Event Sourcing DIY Repo. I wanted to port the Conf Planner to this new backend but I wanted people to get started as easy as possible, so I only added the in memory Event Store. The postgres implementation I did over there kind of works but was never really tested. If we wanted to add it here we need a way of explaining what needs to be done in order to get it working. Otherwise, its just dead code that nobody uses. I am not sure why I only commented this stuff out and did not get rid of it completely.

So if we re-add it, we also need documentation what needs to be done to actually use it.

What do you thing?

CallumVass commented 4 years ago

Hi Roman,

Thanks for the reply, I wasn't aware the code was taken from your DIY Repo.

I agree that documentation should probably be added to provide better information about the different implementations. I'm happy to get started on that in this PR if you wish? Or we can have a discussion around how the documentation should look?

I'm thinking along the lines of having a quick start which will use the InMemory/File implementation and then a deep dive section which will explain how to add database persistence to the app.

Am I right in thinking that a Postgres ReadModel will also need to be implemented? At the moment I only saw an InMemory one? Again I'm happy to add that in this PR if that is the case?

Finally, we probably need a better way of just switching the implementations more easily in the WebServer file

rommsen commented 4 years ago

Hey Callum, sorry for the slow replies. It is pretty busy here at the moment and we still have two kids at home and two full time jobs to handle.

Yes if we are adding the postgres side, we need to add documentation and a nice way to switch. I am happy to incorporate this in this repo. I just dont want an unusable and undocumented option lying around.

A postgres readmodel should also be in there, you are right. You can see an example here: https://github.com/rommsen/EventSourcing-DIY/blob/master/EventSourced/Application.fs#L72

If you want to work on it, I am happy to give you some feedback and I will add this to the repo. I am just a bit slow for now.

CallumVass commented 4 years ago

Hi Roman,

No worries I understand, I'm in the same situation except with 1 child 😅

Thanks for the tip regarding the postgres read model, I will take a look and try incorporate it into this app.

Additionally I will make a start on documentation to help users get up and running / switch implementations easily!