Extension methods for using programmatic config #1623
Conversation
| } | ||
| } | ||
|
|
||
| private static IEnumerable<NodeConfiguration> GetDefinedNodeConfigurations(this ClusterConfiguration config) |
There was a problem hiding this comment.
Since the property is named Overrides, I think it'd be less confusing if we stick to that: GetOverrides or GetOverrideConfigurations instead of GetDefinedNodeConfigurations and UpdateOverrides orUpdateOverrideConfigurationinstead ofApplyToAllNodes`.
There was a problem hiding this comment.
The point is that this gets the "Defaults" node, as well as the defined overrides
There was a problem hiding this comment.
I didn't realize Defaults is part of Overrides. Hmm. Does that even make sense?
There was a problem hiding this comment.
BTW, here is an example of how I'm using it: https://github.com/jdom/orleans/blob/refactoring-testing-host/test/Tester/DuplicateActivationsTests.cs#L22
There was a problem hiding this comment.
I guess it's fine - we can have it as GetDefinedNodeConfigurations. This is for testing anyway. Maybe move it to Tester then, so that it doesn't pollute the public API?
There was a problem hiding this comment.
no, Defaults is not part of overrides, that's the whole point. This applies the changes to both "Defaults" and the currently defined configuration overrides
There was a problem hiding this comment.
I did not know Overrides was used exclusively in tests, then yes. I think moving to TestingHost project (instead of Tester) might be appropriate.
There was a problem hiding this comment.
Technically, Overrides can be used elsewhere. Practically, they were needed for the case of a single static config file for multiple silos. With the move to programmatic config, the point becomes moot.
|
I think it's fine. I don't have a strong opinion on any of these questions. |
|
Moved the Apply method to TestingHost project. Now there's no question that it needs to be an extension method :) |
A few useful methods I found while doing a refactoring of the test infrastructure, which uses programmatic config as a first class citizen (PR coming soon, but not just yet)