rebus-org / Rebus.AzureServiceBus

:bus: Azure Service Bus transport for Rebus
https://mookid.dk/category/rebus
Other
33 stars 20 forks source link

WIP: Sessions support #92

Closed georgechond94 closed 11 months ago

georgechond94 commented 1 year ago

Rebus is MIT-licensed. The code submitted in this pull request needs to carry the MIT license too. By leaving this text in, I hereby acknowledge that the code submitted in the pull request has the MIT license and can be merged with the Rebus codebase.

georgechond94 commented 1 year ago

Hi @mookid8000! Is this something we still consider merging? I fixed some of the tests, but there are still some that timeout, mostly because of the blocking Receive.

Any help would be appreciated.

georgechond94 commented 1 year ago

Regarding c3dd90ac29c0be70579daebbe99e1141ab14a3a8, I am not sure if that is still needed because of the Receiver -> Processor change. ServiceBusProcessor.DisposeAsync() mentions:

Performs the task needed to clean up resources used by the ServiceBusProcessor. Any in-flight message handlers will be awaited. Once all message handling has completed, the underlying links will be closed. After this point, the method will return. This is equivalent to calling CloseAsync(CancellationToken).

Filipedguy commented 1 year ago

@mookid8000 @georgechond94 We still want to merge this? We would love to have session support

mookid8000 commented 11 months ago

I have never been a fan of ASB sessions because they promise more than can be kept when using Rebus and factoring in error queues and possibly Fleet Manager. Also I think the failing tests hint at something being non-trivial about adding this feature.

If session support is really wanted, I suggest you maintain a fork with it. If it proves valuable and does not break other things, we can talk about merging to mainline again.

mookid8000 commented 11 months ago

Oh and I'm sorry, but I actually thought I had closed this PR long ago. Don't know how I missed it for so long 😐 sorry again

georgechond94 commented 11 months ago

Ah that's a pity. I am using my fork on production for a while now and it works as expected. Although, the use case is pretty basic.

Hopefully we will reconsider merging that in the future.