search

houseofcat / turbocookedrabbit

A user friendly RabbitMQ library written in Golang.
MIT License
107 stars 20 forks source link

Move from streadway to official amqp091 go module #32

Open tiggerite opened 2 years ago

tiggerite commented 2 years ago

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.

houseofcat commented 2 years ago

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

I have to potentially rewrite the connection pool and reconnectivity.

tiggerite commented 2 years ago

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 commented 2 years ago

@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 :)