brefphp / symfony-messenger-sns

[DEPRECATED] Installs and confgures SNS with Symfony Messenger
6 stars 0 forks source link

Support for SQS #3

Open kasperg opened 5 years ago

kasperg commented 5 years ago

Thanks for sharing this @Nyholm. Along with your excellent blogpost it helped me get started with Bref and Symfony Messenger.

I am also looking into Amazon SQS and it seems as if much of the setup would be the same as in this package:

Have you considered if/how SQS could work with this package?

One way would be to create a sister package brefphp/symfony-messenger-sqs. Another would be to expand this package to allow it to work with both queue systems.

Nyholm commented 5 years ago

I’ve not worked with SQS but I’m sure it would be very similar to this package.

Maybe you could make a PR with some changes to this package so it works with SQS. Don’t make the perfect final solution, just do small modification so it works with SQS. (It is fine if it doest work with sns anymore). Now we can clearly see if this should be a separate package OR if we should add some config to this package so it work with both.

Is that a fair way of moving forward?

mnapoli commented 5 years ago

Hey, turns out I'm looking into this too!

AFAICT there is not much difference in the code, but I haven't tested, that's just my impression from looking at it.

mnapoli commented 5 years ago

And I was starting a new issue to discuss this but it's actually related to SQS:

I think we should not catch exceptions in Consumer:: consume(). At least for SQS, maybe not SNS.

The reason is that the Lambda SQS integration works like this:

If we always catch exceptions, we loose failure tracking and the features that go with it.

However I'm not sure about SNS. I think it is possible to have Lambda automatically put failed SNS messages into a SQS dead letter queue, which would support doing the same for SNS. But I'm not sure at all. I've read your article (http://developer.happyr.com/sns-retry) @Nyholm but it handles failures manually: if there's a native way to do that that might be a better (simpler and safer) option.

kasperg commented 5 years ago

I think we should not catch exceptions in Consumer:: consume(). At least for SQS [...]

Agreed. Without having looked deeper into SQS or the Enqueue transport I would expect the Messenger retry strategies to work for handling of lambda failures.

How would you prefer that I proceed with this? I am fine with following the process outlined by @Nyholm but if you already have something cooking @mnapoli I am happy to take a look at that as well.

mnapoli commented 5 years ago

I am working on this at the moment, I'll share once I have something working.

mnapoli commented 4 years ago

Hey! I have released https://github.com/brefphp/symfony-messenger-sqs as a separate package.

The reason I created a separate package is that it works very differently. I tried to integrate SQS as described here, using the native Lambda-SQS behavior.

Symfony does not poll SQS, instead we rely on all the features AWS provides around SQS-Lambda. That also means that many features or Messenger are rendered obsolete (e.g. the retry count, error handling, etc.).

I think we could turn https://github.com/brefphp/symfony-messenger-sqs into a generic package that supports SQS, SNS and EventBridge. I worked with all 3 and they work very similarly here.

Let me know what you think.

kasperg commented 4 years ago

I have not played around with it yet but based on the README it looks right to me and the code seems nice and simple.

One question: This package provides a Consumer.php handler. When using brefphp/symfony-messenger-sqs you have to create your own. Is there a reason for not providing an actual implementation in the package?

mnapoli commented 4 years ago

@kasperg yes that's a very good point! I haven't dived into Symfony flex recipes, I kind of gave up from the start with that because AFAIK you have to submit them to a 3rd party repository and you have no control whether they will be accepted or not. It also makes maintenance harder.

But it would be an improvement to do what is done in this repository.

mnapoli commented 4 years ago

Also I had to create the SQS package to be able to build it and distribute it quickly to a client, but now that it's working well I think we can go a step further: we could merge both repositories into one bref/symfony-messenger package that could support SQS, SNS and EventBridge.

But first I'd love to know what you think of the approach in that new package @Nyholm and see if that's the way we would want to go.