Skip to content
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

Symfony Flex Recipe #41

Open
codeliner opened this issue Oct 28, 2017 · 25 comments
Open

Symfony Flex Recipe #41

codeliner opened this issue Oct 28, 2017 · 25 comments

Comments

@codeliner
Copy link
Member

We should think about adding a recipe for the bundle to the recipes contrib repo

@juliusstoerrle
Copy link

Is there any configuration needed, which could be done by the recipe?

@UFOMelkor
Copy link
Member

To be honest I don't see anything that might be configured out of the box.
But how about providing a prooph/pdo-event-sourcing-pack which

  • installs prooph/service-bus-symfony-bundle
  • installs prooph/event-store-symfony-bunde
  • installs prooph/event-sourcing
  • installs prooph/event-store-bus-bridge
  • installs prooph/pdo-event-store
  • configures both bundles with one default bus / event store (mysql?)
  • adds the services that are necessary to connect event-store and service-bus (aggregate translator, event-store, event-publisher etc)

This would allow beginners to quickly dive into prooph without much knowledge.

Link prooph/event-store-symfony-bundle#26

@codeliner
Copy link
Member Author

@UFOMelkor great idea, really like it and yes MySql should be the default because reasons ;)

@gquemener
Copy link
Contributor

gquemener commented Mar 19, 2018

Hello there!

I'm currently digging into Symfony flex recipe creation and wanted to give it a try for Prooph!
So far, my conclusion is that, in order to achieve what's described here, it would require to:

  1. Create a repository for the prooph/pdo-event-sourcing-pack containing a composer.json with the dependencies described by @UFOMelkor (1).
  2. Create a PR on https://github.com/symfony/recipes-contrib with a manifest for each repository containing the necessary configurations.

Is there another on-going initiative which covers this issue?
I'll keep you posted.

(1) This is inspired by https://github.com/php-http/httplug-flex-pack, I haven't found documentation about creating a flex pack yet.

@gquemener
Copy link
Contributor

gquemener commented Mar 19, 2018

When someone will run composer require prooph/pdo-event-sourcing-pack, that would download the composer.json and install these package dependencies (BTW, this package would need to be registered on packagist also).
At this moment, each dependencies would be caught by the Symfony Flex plugin and use the recipes available on the symfony/recipes-contrib repository.

That's how I think it would work, I'm currently reading and trying it out to see if this is really how it'll behave.

If someone has more xp about it, I'd be happy to share some thoughts!

@gquemener
Copy link
Contributor

Heads up:

  1. I've created a "pack" repository containing the mentionned dependencies (ofc, I'd be happy to change ownership if this whole thing goes through!): https://github.com/gquemener/prooph-pack/
  2. I've created a PR on symfony/recipes-contrib to start experimenting: Added recipe for the Prooph pack symfony/recipes-contrib#320

@gquemener
Copy link
Contributor

Hello,

I have a confrmation that the flex recipe mecanism works as I believed it would.

Here's what I get so far:

$ SYMFONY_ENDPOINT=https://symfony.sh/r/github.com/symfony/recipes-contrib/320 composer.phar require "gquemener/pdo-event-sourcing-pack:*@dev"
Warning: Using https://symfony.sh/r/github.com/symfony/recipes-contrib/320 as the Symfony endpoint
./composer.json has been updated
Warning: Using https://symfony.sh/r/github.com/symfony/recipes-contrib/320 as the Symfony endpoint
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 13 installs, 0 updates, 0 removals
  - Installing beberlei/assert (v2.9.3): Loading from cache
  - Installing paragonie/random_compat (v2.0.11): Loading from cache
  - Installing ramsey/uuid (3.7.3): Loading from cache
  - Installing prooph/common (v4.2.1): Loading from cache
  - Installing marc-mabe/php-enum (v3.0.0): Loading from cache
  - Installing prooph/event-store (v7.3.2): Loading from cache
  - Installing prooph/pdo-event-store (v1.7.2): Loading from cache
  - Installing prooph/service-bus (v6.2.2): Loading from cache
  - Installing prooph/event-store-bus-bridge (v3.1.0): Loading from cache
  - Installing prooph/event-sourcing (v5.3.0): Loading from cache
  - Installing prooph/event-store-symfony-bundle (v0.4): Loading from cache
  - Installing prooph/service-bus-symfony-bundle (v0.6.0): Loading from cache
  - Installing gquemener/pdo-event-sourcing-pack (dev-master 2f8347f): Loading from cache
Writing lock file
Generating autoload files
Symfony operations: 2 recipes (117325007f3b3255fa1de7ad69822929)
  - Configuring prooph/event-store-symfony-bundle (>=v0.4): From auto-generated recipe
  - Configuring prooph/service-bus-symfony-bundle (>=0.6): From github.com/symfony/recipes-contrib:320
Executing script cache:clear [OK]
Executing script assets:install --symlink --relative public [OK]

Some files may have been created or updated to configure your new packages.
Please review, edit and commit them: these files are yours.

And a command bus (named default_command_bus) is preconfigured.

@prolic or @codeliner, could you create a repository under the prooph org against which I could PR the content of https://github.com/gquemener/prooph-pack please?

@codeliner
Copy link
Member Author

@gquemener repo is here: https://github.com/prooph/prooph-pack

@prooph/symfony-team has write access ^^

@UFOMelkor
Copy link
Member

@codeliner Perhaps pdo-event-sourcing-pack is a better name?

@sandrokeil
Copy link
Member

@codeliner Maybe pdo-event-sourcing-symfony-recipe.

@codeliner
Copy link
Member Author

@sandrokeil looks like the recipes are called "pack", see httplug-pack

@codeliner
Copy link
Member Author

@UFOMelkor @gquemener

Perhaps pdo-event-sourcing-pack is a better name?

Ok so better two packs instead of one?

  • prooph/pdo-event-sourcing-pack
  • prooph/service-bus-pack
    ?

@gquemener
Copy link
Contributor

gquemener commented Mar 19, 2018

As far as I undeserstand,
pack is a set of dependencies.
recipe is a set of rules applied by the symfony flex composer plugin when a specific dependency is being installed.

Technically, it's possible to not rely on pack, but they are quite useful for the users who want a solution "out-of-the-box".

Anyway, I've started to focus on the service-bus-symfony-bundle recipe.
Here's what I thing the recipe should provide as a basic configuration:

  1. Register the Prooph\\Bundle\\ServiceBus\\ProophServiceBusBundle in all environments
  2. Register a default_command_bus command bus
  3. Create an alias named Prooph\Bundle\ServiceBus\CommandBus to the service prooph_service_bus.default_command_bus (in order to enable autowiring)
  4. Autoregister command handlers located in src/Messaging/Command/*Handler.php with message_detection enabled
  5. Do the same thing as 2, 3 and 4 for a default event bus
  6. Do the same thing as 2, 3 and 4 for a default query bus

I don't think it's quite possible to do something like "register an event bus only when the event-sourcing pack is required". The same recipe will be applied when the bundle is installed, no matter which pack requires it (I could be wrong though...).

I haven't put my fingers in the components for a few months, so I'd like your thoughts about these features, please 👍

@gquemener
Copy link
Contributor

gquemener commented Mar 19, 2018

Point number 4 is problematic because we cannot provide a services.yaml file in the recipe, as it would completely overwrite the one provided by symfony/framework-bundle (and we do not want that!).

From then, our only option to register the command handler services is in a file inside the packages directory.

But, because the default Kernel loads the services.yaml after the packages/*.yml files, our command handler definitions (with the appropriate tag) will be overwritten by the configuration of the framework-bundle services.yaml (which register privately all the classes found in src/), thus removing the tag.

It's a tricky problem, I hope I make sense!

So far, I've found a few solutions to this problem:

Provide a services.yaml which contains the same thing as the framework bundle, except that it also ignores the classes in the Messaging namespace on line 18. We can then provide the auto configuration inside a yaml file in packages and the service definitions will not be overwritten. We could also provide directly the configuration in the services.yaml file.
=> This is a super dirty hacky solution, and would require to update the services.yaml every time the symfony/framework-bundle recipe update its! I wouldn't recommend it.

Provide the command handler definition in a file in inside the packages directory and display a message at the end of the installation stating that user should manually disable any command handler definition overwriting (by excluding the Messaging directory for instance).

Provide nothing and display in a message at the end of the installation the yaml to auto register all command handlers.

What do you think?

@codeliner
Copy link
Member Author

Second option: displaying a message sounds good

But I'm not an expert in this area. Hopefully @UFOMelkor can say more

@gquemener
Copy link
Contributor

After thinking about it, we might be able to provide different configuration sets depending on which pack is required (by defining a recipe for this pack, and not for the bundles contained in the pack),

But, that'd be overkill, don't you think?

I think I'm missing experience on Prooph to know what out-of-the-box configuration would improve DX 🤔

@gquemener
Copy link
Contributor

With my current knowledge, I would forget the packs and provide recipe for the prooph bundles that setup basic configurations (inspired by their Getting started documentation for example).

@codeliner
Copy link
Member Author

what out-of-the-box configuration would improve DX

I've put a lot of thoughts into this question lately and started to work on a beginner friendly ready-to-use package called Event Machine (not symfony related) and a preconfigured skeleton: https://github.com/proophsoftware/event-machine-skeleton

But this package is a framework in itself ^^. It is based on prooph components but uses them in a certain way. A lot of decisions are made upfront so that new users don't need to think about it but just start working with the skeleton and build something awesome.

The downside of this approach is, that you loose flexibility. One example:
Event Machine uses an event store with a single event stream named event_stream. While this is a good option for small to mid-sized projects it does not scale well for systems that have to handle higher load.

Another example is that Event Machine only uses one command bus, one event bus and one query bus for the entire application/service. Again, this works pretty well for mid-sized applications and/or (micro)services, but is not so good for monolithic systems split into code modules.

As far as I understand a recipe tries to do the same thing: Provide some good defaults so that developers can start right away.
But the question is, what is a good default?

For a microservice with its own database the set up described above is really everything you need.
But for a monolithic application it is better to start with dedicated buses and event store configuration for each code module from day one.

How does the doctrine recipe solve the issue?

Maybe the result is, that a good default recipe can not be provided for prooph. Or the majority of symfony devs say that a single bus and an event store with only one stream are a good start.
TBH I don't know.

@gquemener
Copy link
Contributor

I've looked in many recipes to see how one would register services with classes from the App namespace, I haven't found any.

For instance, doctrine only register services from the Doctrine namespace: https://github.com/symfony/recipes/blob/master/doctrine/doctrine-bundle/1.6/config/packages/prod/doctrine.yaml.

Maybe it's a bad idea 🤔

I will take a look on Event Machine tomorrow 👍

@UFOMelkor
Copy link
Member

IMHO such a pack should have two main purposes: It should allow a faster trying out and take copy and paste configuration off my hands.

The actual effort to start with full prooph in symfony is:

  1. Install the following packages (might be a hurdle for beginners):
    • prooph/service-bus-symfony-bundle
    • prooph/event-store-symfony-bunde
    • prooph/event-sourcing
    • prooph/event-store-bus-bridge
    • prooph/pdo-event-store
    • react/promise
  2. Configure one command/event/query-bus. This is actually very easy but a little bit annoying (typic copy paste).
  3. Configure one event store. This partially requires a little bit more thinking. (Which event store do I want to use? Where do I get my connection from?) Possible hurdles for beginners: There are many services that need to be configured just to configure my event store. Why do I need to configure one AggregateTranslator (in 99% the user will use the Prooph\EventSourcing\EventStoreIntegration\AggregateTranslator), why do I need to configure a StreamStrategy (and what is this)?
  4. Marry event-store and services-buses aka adding the TransactionManager and the EventPublisher. This seems really hard to me because there is not enough documentation for it available (and to be honest for trying out I don't care how this works).
  5. Add Aggregates, Commands, Command Handlers, Repositories, ...

(1)-(4) are tasks which are down rarely (once for an application) while (5) is the daily work.
Therefore I would focus on simplify (1)-(4) and not care about (5). Every user should be able to read at least some docs.

But the question is, what is a good default?
[…]
But for a monolithic application it is better to start with dedicated buses and event store configuration for each code module from day one.

Nothing prevents you from adding multiple buses / event stores.
Doctrine configures one database connection by default. Nothing prevents you from adding more connections. It's just a start configuration.

Perhaps pdo-event-sourcing-pack is a better name?

Ok so better two packs instead of one?
prooph/pdo-event-sourcing-pack
prooph/service-bus-pack
?

No, my propose is just one package named pdo-event-sourcing-pack.
I know that the service bus is not necessary for event-sourcing.
But the main audience searching for such a pack would be searching either for prooph (which is already part of the vendor) or for event-sourcing (if they do not know prooph and looking for an ES framework in PHP).
In both cases giving them the complete power of prooph looks fine to me. If they do not want to use the service bus, fine, they can always unpack it.

@gquemener
Copy link
Contributor

gquemener commented Mar 21, 2018

Hello @UFOMelkor,

I completely agree with all you say, that'll be a great way to get started in a symfony / prooph project.

I'd like to add a few things.

Pack vs Skeleton

I'm not sure if the good solution wouldn't be to create a symfony skeleton project, instead of a pack.

Here's an example of skeleton: https://github.com/symfony/website-skeleton/blob/4.0/composer.json.

A Prooph symfony skeleton project would allow to start faster coding:

$ composer create-project prooph/symfony-skeleton my-project
$ cd my-project
$ vim

A pack would require to perform something like this:

$ composer create-project symfony/website-skeleton my-project
$ cd my-project
$ composer require prooph/symfony-pack
$ composer remove orm (+ everything else uneeded)
$ vim

The downside of a skeleton is that I don't think it's made to be required on existing project (whereas symfony packs are), but the main idea here is to get started faster, isn't it?

Bundle-wide vs Pack/Skeleton-wide recipes

It seems that Symfony Flex can execute recipes no matter if the package is a library or a bundle.

However, I think this system has a flaw because it creates recipe dependencies.
For instance, requiring symfony\routing without symfony/framework-bundle, would copy this config directory that contains symfony/framework-bundle configuration into the project config directory.
I'm not sure why the Core Symfony team has done that, I guess they considered that if you're using symfony/flex then you're in a Symfony project that MUST requires symfony/framework-bundle.

In the case of Prooph, if we follow the same example, it would mean creating a prooph\pdo-event-store recipe that would create a mysql event store and register it using prooph\event-store-symfony-bundle configuration (thus making an implicit dependencies between the two).

My suggestion would be to define one skeleton/pack-wide recipe, executed only when requiring the skeleton/pack. The downside is that this recipe wouldn't be executed when requiring packages manually, but is it really a problem?

Another suggestion would be to define recipes ONLY for prooph bundles.
For example, we could provide commented configuration in the prooph/event-store-symfony-bundle recipe to register a pdo event store (with explanation that user MUST require the prooph/pdo-event-store package in order to uncomment this section).

Current status

So far, I've managed to configure a command bus and auto-register command handlers => symfony/recipes-contrib#320.
I've encountered a small configuration issue when registering a repository, some required keys that were not marked as required. I've create a PR here.

I've also realised that prooph service definition still use the old id "convention", new one being to use the FQCN (leveraging autowiring).

This leads me to manually create alias to enable autowiring:

services:
        Prooph\Common\Messaging\MessageFactory: '@prooph_event_store.message_factory'

Is there any reason for that? Should I investigate further and create a PR to switch to FQCN ids?

Last but not least, do you have an idea about what happened to the prooph/event-store-symfony-bundle documentation?
The directory is empty, but the bookdown.json references some unexisting files.

@gquemener
Copy link
Contributor

@UFOMelkor

The actual effort to start with full prooph in symfony is:

You haven't mentionned registering the command, event, query handlers. Do you think it's a bad idea to provide default configuration for this?

@UFOMelkor
Copy link
Member

UFOMelkor commented Mar 21, 2018

Pack vs Skeleton

My personal opinion: Having both would be great, but would double the effort of maintenance.
Having a skeleton only would mean that you cannot easily try it your existing project or in different standard editions (including internal ones of companies).
Another problem of a skeleton: What would be part of is? Twig to render html? FosRest to make an API?

Bundle-wide vs Pack/Skeleton-wide recipes

My suggestion would be to define one skeleton/pack-wide recipe, executed only when requiring the skeleton/pack. The downside is that this recipe wouldn't be executed when requiring packages manually, but is it really a problem?

The only alternative I see is to create a recipe for

  • prooph/service-bus-symfony-bundle
  • prooph/event-store-symfony-bunde
  • prooph/event-store-bus-bridge
  • prooph/pdo-event-store

which all ship their own config and add a pack which just requires the right packages (like the debug-pack does). But this would require more recipes and therefore more configuration files (4) and would come to the same result in most cases.

👍 for one pack with one recipe.

Current status

I've create a PR here.

I'll check tomorrow but first impression looks good :-)

I've also realised that prooph service definition still use the old id "convention", new one being to use the FQCN.

For bundles the old id convention is still actual, but we miss aliases for public services (see https://symfony.com/doc/current/bundles/best_practices.html#services); this leads us to the question which services should be public, but IMHO the MessageFactory is one.

You haven't mentionned registering the command, event, query handlers. Do you think it's a bad idea to provide default configuration for this?

I hid it behind (5): Add Aggregates, Commands, Command Handlers, Repositories, ... 😉

I don't think that it is wrong, but that it won't help very much. Either you have to recognize were to put your handlers and modify this or you have to learn how to configure this (which you easy and you need to learn anyway). Might be that I'm just to informal?

@gquemener
Copy link
Contributor

gquemener commented Mar 21, 2018

My personal opinion: Having both would be great, but would double the effort of maintenance.

Allright, maybe when there will be more maintainers on the project then 😉

👍 for one pack with one recipe.

Let's go 👍

For bundles the old id convention is still actual

I skipped this part, thanks for pointing that out.
👍 to create the public alias in the event-store-symfony-bundle (PR: prooph/event-store-symfony-bundle#45)

Either you have to recognize were to put your handlers and modify this or you have to learn how to configure this (which you easy and you need to learn anyway).

Indeed.
Still, I would suggest to add a commented section in the configuration to show how easily you can register them.

@UFOMelkor
Copy link
Member

Allright, maybe when there will be more maintainers on the project then 😉

Would be great 👍

Still, I would suggest to add a commented section in the configuration to show how easily you can register them.

Good idea 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants