swarrot / SwarrotBundle

A symfony bundle for swarrot integration
MIT License
89 stars 59 forks source link

Default middleware stack #100

Open greg0ire opened 7 years ago

greg0ire commented 7 years ago

I think it would be handy to be able to define a default middleware stack that would server as a basis for every consumer. I discovered the swarrot.processor.ack and I can't imagine why one would ever not use it. I can provide a PR for that.

odolbeau commented 7 years ago

To be honest, I don't know. I also use this processor with most of my consumers. I also use near everytime the same middleware stack (retry, signal handler & so on). At the end, the configuration I use is very similar to the reference configuration which could be the default one. My default one in fact. If you don't use one of the processor I use, you'll have to override the whole middleware stack. If we had this processor (and only this processor) in the default middleware stack, I'll this have to override this config to fit my needs. As you can see, I don't think it's that useful to define a default middleware stack which will be override by most of swarrot users anyway.

greg0ire commented 7 years ago

@odolbeau I don't mean to contribute a default middleware stack. I want to give you the possibility to define your middleware stack once, and reuse it as a basis (meaning you could add to it).

The configuration could look like this :

Solution 1 "default" or "base" middleware stack

swarrot:
    default_middleware_stack:
        - configurator: swarrot.process.ack
        - configurator: another.processor
        - configurator: you.get.the.idea
    consumers:
        no_additional_processor_consumer:
            processor: innermost_processor
        consumer_with_extra_processor_at_the_top_of_the_stack:
            processor: innermost_processor
            middleware_stack:
                - configurator: additional_configurator
        consumer_with_extra_processor_at_the_bottom_of_the_stack:
            processor: innermost_processor
            middleware_stack:
                - configurator: additional_configurator
                - configurator: swarrot.process.ack
                - configurator: another.processor
                - configurator: you.get.the.idea

Another potential improvement would be to allow the definition of several middleware stacks, and merge them

Solution 2 : reference any number of middleware stacks and merge them together

swarrot:
    middleware_stacks:
        arbitrary_name:
            - configurator: swarrot.process.ack
            - configurator: another.processor
            - configurator: you.get.the.idea
        whatever:
            - configurator: additional_configurator
    consumers:
        no_additional_processor_consumer:
            processor: innermost_processor
            middleware_stack:
                - arbitrary_name
        consumer_with_extra_processor_at_the_top_of_the_stack:
            processor: innermost_processor
            middleware_stack:
                - arbitrary_name
                - whatever
        my_third_consumer:
            processor: innermost_processor
            middleware_stack: # size of the stack: 5
                - arbitrary_name
                - whatever
                - configurator: extra_configurator # BC-compatibility is
                  # achieved by looking at the contents of the array : if it is a
                  # simple string, it is a reference to a middleware_stack, if
                  # it is a hash, then the definition is inline
odolbeau commented 7 years ago

In this case, I don't really understand what you want to do. Do you use the same middleware stack for different consumers? If it's the case, why did you choose to use different consumers?

greg0ire commented 7 years ago

@odolbeau I'm doing very different jobs, based on the same kind of message => I need several queues consumers, it's the best practice if we believe this article : https://derickbailey.com/2015/09/02/rabbitmq-best-practices-for-designing-exchanges-queues-and-bindings/ .

If you need specific messages to go to specific consumers, you do this through the routing and bindings. Trying to filter once the message is in the queue, is an anti-pattern in RabbitMQ.

Likewise, trying to route the message to a specific service once it is in the consumer seems like an anti-pattern to me.

I also plan to add to a new consumer, that will consume a very different kind of message. In that case, I will also need a separate consumer, but in all cases, I will use the swarrot.processor.ack processor, and I'd like to be able to express that in a DRY way.

greg0ire commented 7 years ago

I have a slight preference for the first solution, the second one might be overkill, and is more verbose, although more flexible. What do you think?

greg0ire commented 7 years ago

Do you use the same middleware stack for different consumers?

To clarify, yes, but the innermost processor (the one I write) is not necessarily the same. I say not necessarily because I also happen to have two consumer with exactly the same stack, but a different vhost : there is a "real vhost", from where legit events come (produced by a legacy django app), that I have read-only access on, and a "local vhost", that produces similar events, so that I can do a bulk import by producing many events if I need to (from my non-legacy symfony app).

odolbeau commented 7 years ago

If I understand well, you would like to have a new configuration key like default_middleware_stack which will be used by default for all your consumers. As soon as you change this stack, you have to override it for the consumer you want? So it will be useful only if you have n consumers with the exact same middleware stack?

Did I understand well?

greg0ire commented 7 years ago

If I understand well, you would like to have a new configuration key like default_middleware_stack which will be used by default for all your consumers.

Yes, that's the first solution I described.

As soon as you change this stack, you have to override it for the consumer you want?

You could complement it on a consumer-per-consumer basis, but you wouldn't be able to override it. So if there is an item you want for 90% of your consumers, you can't add it to the default stack (which would maybe better be named as base_stack ?)

So it will be useful only if you have n consumers with the exact same middleware stack?

It would be useful only if you have n consumer with the exact same base stack :

- consumer1:
    - my_processor_that_does_a_specific_job_that_I_wrote_in_my_project # innermost processor
    - ackprocessor
    - memory_processor
    - retry_processor
    - signal_handler_processor
    - some_other_processor
- consumer2:
    - schwift_schmeckles_processor                       # innermost processor
    - ackprocessor                                       ^             ^
    - memory_processor                                   |             | base
    - retry_processor                                    | middleware  | middleware
    - signal_handler_processor                           | stack       V stack
    - some_other_processor_that_I_want_here              |
    - some_other_processor_that_I_want_here_too          V
- consumer3:
    - copy_photos_processor                              # innermost processor
    - ackprocessor
    - memory_processor
    - retry_processor
    - signal_handler_processor

Here, the default_stack or base_stack would be

    - ackprocessor
    - memory_processor
    - retry_processor
    - signal_handler_processor

It may or may not be augmented with other processors. With solution 1, you can't define a consumer that does not have the memory processor, for instance.

Also, for me, after reading your docs, especially the config part, I think the innermost job my_processor_that_does_a_specific_job_that_I_wrote_in_my_project is not part of the middleware stack, hence maybe the confusion.

greg0ire commented 7 years ago

Solution 3: chain configurator

swarrot:
    chain_configurator:
        default: # can be any string, if not empty, a service instance of
                 # ChainConfigurator will be created (swarrot.processor.chain.default)
            - configurator: swarrot.process.ack
              extras:
                  requeue_on_error: false
            - configurator: another.processor
            - configurator: you.get.the.idea
    consumers:
        no_additional_processor_consumer:
            processor: innermost_processor
            middleware_stack:
                - configurator: swarrot.processor.chain.default
        consumer_with_extra_processor_at_the_top_of_the_stack:
            processor: innermost_processor
            middleware_stack:
                - configurator: swarrot.processor.chain.default
                - configurator: additional_configurator
        consumer_with_extra_processor_at_the_bottom_of_the_stack:
            processor: innermost_processor
            middleware_stack:
                - configurator: additional_configurator
                - configurator: swarrot.processor.chain.default
odolbeau commented 7 years ago

As discussed with @greg0ire, the latest solution looks like the best one. Even if we're not sure it's a common use case to have several chain_configurator, it allows us to ship some default chained configurator with the bundle if we want.

Furthermore, compared to other solutions, the middleware stack definition stay unchanged, there is only a new part in the config to declare chain configurators.

rmasclef commented 6 years ago

@odolbeau any update on this feature ?

odolbeau commented 6 years ago

@olaurendeau what do you think?

duraki commented 6 years ago

Stumbled upon this issue since I wanted to "beauty" my yaml. This really needs to happen, config file with many consumers looks rather dirty.

greg0ire commented 6 years ago

@duraki would you like to contribute it?