-
Notifications
You must be signed in to change notification settings - Fork 23
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
Naming convention #20
Comments
Hi, PR's are always welcome and a mongoDB adapter is something I have on my todo for long time now. Btw I've seen your HumusAmqpModule 👍 |
I could also imagine building that amqp integration for you. But first of all, I need to do some improvements on the amqp module itself and release 1.0.0 ;-) |
For the mongodb adapter: I can also imagine modelling the documents slightly different. Instead of having all events mapped as embedded objects of the aggregate, simply persist all events to a distinct event collection. |
👍 I'm looking forward to the implementation. I will create the repo tomorrow and post the link here. Now it's time to shutdown my system :-) Thank you for your help and input. I really appreciate it. |
@prolic empty mongodb adapter repo is online: https://github.com/prooph/event-store-mongodb-adapter |
Great thanks! Two questions come into my mind first: a) Do we make a pure mongodb adapter or a doctrine-mongodb adapter? I really like the abstraction doctrine-mongodb provides!
If we do something like this, which TTL will we use? MySQL InnoDB has a innodb_lock_wait_timeout of 50 secs by default f.e. About maximum document size: We can perhabs have 2 adapters? |
@prolic I've split your questions into issues ^^ so that we can discuss them individually |
crazy, this issue is still not closed. We should keep it open for historic reasons :D |
Hi,
first of all, many thanks for this great library. Expect some PR's in near future from me. (MongoDB EventStore Adapter could be the first!)
On thing I noticed during code review:
class Prooph\Proophessor\EventStore\AbstractRepositoryFactory
The name hints for an abstract class, but it's an abstract service factory (it should not get extended). So a better class name would be RepositoryAbstractFactory instead of AbstractRepositoryFactory.
Merely a small detail regarding the whole thing ;)
Keep on doing great stuff!
The text was updated successfully, but these errors were encountered: