[GitHub] [geode] kohlmu-pivotal commented on pull request #5739: GEODE-8697: Propagate ForcedDisconnectException to the user application

[GitBox <gi...@apache.org>]

kohlmu-pivotal commented on pull request #5739:
URL: https://github.com/apache/geode/pull/5739#issuecomment-727675133


   Whilst I cannot comment on the logic that, I do always wonder why we keep on supporting test code inside of production classes. Thinking about it, the whole "testHook" logic should live in a test class that extends the `ClusterDistributionManager`. In this test, there would be no problem doing this. Whilst it would require some refactoring of the root class to allow for the extension, I don't think it would be that significant that this cannot be achieved.
   
   Also, given the fact that there is only one location where a `ClusterDistributionManager` is created, I imagine it would be only a small refactor to have an ability to pass in a DistributionManager into the `InternalDistributedSystem` in order to have the ability to switch to another DistributionManager. It might actually even simplify some of the logic in the `InternalDistributedSystem`.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org