search

graze / queue

:postbox: Flexible abstraction for working with queues in PHP.
MIT License
49 stars 10 forks source link

Adding queue name to FailedEnqueueException [1.0] #37

Closed wpillar closed 7 years ago

wpillar commented 7 years ago

FailedEnqueueException isn't hugely useful at the moment, if I'm a client and catching it, I have little idea of what's gone on. I can pull out the failed messages and the adapter that emitted it but not the queue url/name that failed to enqueue.

This PR adds an argument to FailedEnqueueException to represent the name of the queue. In SqsAdapter I have simply used the $this->name var for this purpose. I don't think it's unreasonable to assume that other queue adapters will also have a unique identifier for a queue that can be used.

wpillar commented 7 years ago

@h-bragg as it stands this is a BC break, do you agree? I could change it to be a minor rather than major change. Thoughts?

h-bragg commented 7 years ago

Hmmm, that is true. For something this minor it seems excessive to generate a whole new major(ish) version.

You could do some __toString() checking; a bit ugly. OR some kind of NamedInterface or QueueNameInterface that has getName() which the adapter might implement.

Perhaps we should do 2 PR's one (this change) and merge into a new 0.4 branch and another that is the equivalent but only a minor change to get the name but a bit more hacky.

wpillar commented 7 years ago

@h-bragg this is now a BC break PR into the 1.0 branch.

h-bragg commented 7 years ago

So, with the other change... Should we actually keep this. Using a NamedInterface seems a bit tidier, and with AdapterException being the base it is more generic...

wpillar commented 7 years ago

@h-bragg yeh having gone through the other change, I think it's a better all-rounder. Close this off?

wpillar commented 7 years ago

Closed because this is a better and less breaking implementation: https://github.com/graze/queue/pull/38