eric-maynard opened a new pull request, #1295: URL: https://github.com/apache/polaris/pull/1295
Created this PR to get feedback on an idea I had while trying to add a new configuration where we'd like to distinguish between "set" and "not set" without just setting the default value to `null` (which won't work for some types). I don't love that this implementation requires `get` to be spammed everywhere. With that said, I think the current way of getting configs via the lengthy invocation `callContext.getPolarisCallContext().getConfigurationStore().getConfiguration(callContext.getPolarisCallContext, FeatureConfiguration.FEATURE_NAME)` should probably be made more ergonomic anyway and that adding `get` to the end doesn't make it too much more painful. We could potentially refactor out the `get`s if/when we refactor the rest. I leave them in here so reviewers can get a good sense of the scope of the change. Critically, this implementation does mean that the caller of `getConfiguration` is expected to know when it's safe to use `get`, i.e. when a config is optional or not. An alternative I considered is to make some new type like `OptionalConfiguation` and to provide new overrides of the `getConfiguration`-like methods for this new type. In that way, the caller could be somewhat shielded from having to worry about whether a config is optional. However, the config code is already pretty gnarly with multiple methods apparently doing the same thing, and I'm worried about adding more complexity there. -- 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]
