search

messaginghub / pooled-jms

A JMS Connection pool for messaging applications supporting JMS 1.1 and 2.0 and Jakarta JMS clients
Apache License 2.0
50 stars 24 forks source link

Clarify options that were recently removed #12

Closed snicoll closed 6 years ago

snicoll commented 6 years ago

In an attempt to migrate Spring Boot auto-configurations from activemq-pool to this project, I've realized a few properties where removed (apparently in this commit that doesn't reference an issue so can't comment there sorry.

We're still considering migrating to this project for the JMS 2.0 API compatibility but it would be nice to get some more information as why those properties were removed so that we can properly deprecate them.

Thanks!

tabish121 commented 6 years ago

The first thing to note here is that this pool is not intended to be a drop in replacement for the ActiveMQ JMS pool and as such is not bound by legacy features or configuration.

Prior to the 1.0 release of this component a thorough review was done in regards to the available configuration options and their usefulness and whether or not it was advisable for users to be able to make changes to certain aspects of the behaviour controlled by said options. For those options that were deemed ill-advised to continue supporting the option was removed to avoid confusion and simplify the user experience. For others the names were changed from the hap-hazard legacy naming to a more consistent and understandable value and documentation updated to make their use more clear.

snicoll commented 6 years ago

Thanks for the feedback but that's essentially what you wrote in the commit message. Is there a way to have more information about why some options were removed?

I never thought this library was a drop-in replacement but I need some motivation to deprecate properties that we're currently exposing.

gemmellr commented 6 years ago

From what I see / recall, the removed options were createConnectionOnStartup (which defaulted to true), expiryTimeout (which defaulted off), and reconnectOnException (which defaulted to true).

The first thing you tend to do with the pool is create a connection, and its presumably being used due to doing that a lot, so its wasnt clear there was real benefit to createConnectionOnStartup and the same effect could be achieved outwith the pool if needed. The expiryTimeout functionality didnt seem that useful and was perhaps confusing alongside the idleTimeout option. The reconnectOnException option seemed odd as disabling it was likely to leave broken connections within the pool.

snicoll commented 6 years ago

Thanks for the feedback @gemmellr!