cshannon commented on PR #2553: URL: https://github.com/apache/activemq/pull/2553#issuecomment-5603209033
> I have a concern that changing the exception handling would be breaking for JMX clients that already handle the exception being thrown. > > I’ll do a couple of checks > > I prefer this clean/empty data approach over throwing exceptions, but it may need to wait to go into a major version. I think this change is probably fine for something like 6.4.0. It also depends on if we want to use the empty data approach instead of error if broker is null no matter what (like the existing PR) or limit it to only if in slave mode. (the broker object is not set until start is called so i'm not sure if it's possible for the broker to be null briefly during initialization). I suggested to do that extra check but maybe we just leave things as is, not sure. > @mattrpav that makes totally sense. Maybe we should target just 6.4.0 for this change (not 7)? +1 for 6.4.0 I think this change is probably fine for something like 6.4.0. It also depends on if we want to use the empty data approach instead of error if broker is null no matter what (like the existing PR) or limit it to only if in slave mode. (the broker object is not set until start is called so i'm not sure if it's possible for the broker to be null briefly during initialization). I suggested to do that extra check but maybe we just leave things as is, not sure. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] For further information, visit: https://activemq.apache.org/contact
