Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify options that were recently removed #12

Closed
snicoll opened this issue Jul 30, 2018 · 4 comments
Closed

Clarify options that were recently removed #12

snicoll opened this issue Jul 30, 2018 · 4 comments

Comments

@snicoll
Copy link

snicoll commented Jul 30, 2018

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
Copy link
Contributor

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
Copy link
Author

snicoll commented Jul 30, 2018

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
Copy link
Contributor

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
Copy link
Author

snicoll commented Jul 31, 2018

Thanks for the feedback @gemmellr!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants