turbocookedrabbit icon indicating copy to clipboard operation
turbocookedrabbit copied to clipboard

Move from streadway to official amqp091 go module

Open tiggerite opened this issue 2 years ago • 4 comments

tiggerite avatar May 18 '22 10:05 tiggerite

Oops, completely missed https://github.com/houseofcat/turbocookedrabbit/issues/17 - will convert this to a draft PR instead @houseofcat so you can have it as reference for when (if?) you do the migration in v3. Would appreciate a look at my other PR though (they are separate entities), it doesn't contain breaking changes unless the Connect return type counts.

tiggerite avatar May 24 '22 19:05 tiggerite

There is internal behavior changes and they promise to break more API naturally.

I have to potentially rewrite the connection pool and reconnectivity.

houseofcat avatar May 24 '22 22:05 houseofcat

There is internal behavior changes and they promise to break more API naturally.

I have to potentially rewrite the connection pool and reconnectivity.

These are fair points. Perhaps ConnectWithErrorHandler would be better as it does not change the current API (only add a new one)? Not sure how to get around internal behaviour changes though without having a handler specifically for (re)connection.

Should I raise an issue for this, as I think having visibility of reconnection errors is important (otherwise you have no feedback when it has been successful) and being able to handle connection errors in general is a prerequisite for metrics/capturing logs with proper formatting etc?

tiggerite avatar May 25 '22 07:05 tiggerite

@houseofcat Ok, I've reworked the PR to not break any existing APIs or internal logic, instead adding a new handler with its own creation mechanism on the connection pool, a new mechanism for creating a rabbit service with a connection pool, and reverting Connect to just return bool with a new ConnectWithErrorHandler function on connection host to handle errors (which Connect calls with no error handler). Please let me know if this is acceptable :)

tiggerite avatar May 25 '22 08:05 tiggerite