Support for module-based read-only commands #2401

[JimB123]

Make sure that:

• You have read the contribution guidelines.
• You have created a feature request first to discuss your contribution intent. Please reference the feature request ticket number in the pull request.
• You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
• You submit test cases (unit or integration tests) that back your changes.

Mark,

Looking for feedback on this. At this point, I've authored the code changes, but no unit/integration tests have been written.

A few questions:

1. Is this what you were envisioning? If not, what needs to be improved?
2. I'm not completely happy with the extraReadOnlyCommands. I'd really rather do something that also pulls in the standard read-only commands and combines the functionality directly. I also wasn't sure that this is what you were expecting.

• I thought about overriding ClientOptions logic in the ClusterClientOptions to automatically pick up the cluster read-only commands - however from the existing code in ClusterDistributionChannelWriter (the constructor), it seems that we can't trust that the ClusterClientOptions is guaranteed to be used. I the end, I decided that some "extra" commands added in ClientOptions - with logic remaining the the channel writers was the most straightforward.

3. What do we need in terms of test cases?
4. I had to change some of the existing test cases, are these changes OK? Or is there a better way to alter them?

[mp911de]

Thanks Jim.

I took a look and addressed

3. I'm not completely happy with the extraReadOnlyCommands. I'd really rather do something that also pulls in the standard read-only commands and combines the functionality directly. I also wasn't sure that this is what you were expecting.

I'm pretty happy with the changes. With a bit of polishing we're quite there with a composable model to easily pick up default opinions and alter these or to let applications take full control over read-only routing.

[mp911de]

Thank you for your contribution. That's merged, polished, and backported now.