adutra commented on PR #1124:
URL: https://github.com/apache/polaris/pull/1124#issuecomment-2704282942
My general feeling about `FeatureConfiguration` is that it's too schemaless.
I don't think this PR addresses that, but I think it's something that needs to
be addressed soon.
A typical configuration class in a Quarkus application would like like this:
```java
@ConfigMapping(prefix = "polaris.feature")
interface FeatureConfiguration {
boolean configX();
Optional<String> configY();
List<String> configZ();
}
```
However, we currently have this:
```java
@ConfigMapping(prefix = "polaris.feature")
interface FeatureConfiguration {
Map<String, String> defaults();
}
```
Where the map values are dynamically parsed as json, then each map entry is
collated against the fields in `PolarisConfiguration`:
```java
public class PolarisConfiguration<T> {
public static final PolarisConfiguration<Boolean> CONFIG_X = ...
public static final PolarisConfiguration<String> CONFIG_Y = ...
public static final PolarisConfiguration<List<String>> CONFIG_Z = ...
}
```
This makes the code quite brittle, as it's not possible to statically
compile it against an existing method like `FeatureConfiguration.configX()`.
I understand that the goal is to be able to override things on different
levels, by realm for instance – and I think that's a great idea. But there are
other ways to achieve that while preserving a strongly-typed configuration
mechanism.
Let's discuss this at the next meeting, but I would suggest to refactor this
by grouping the configuration options into logical groups inside one or many
configuration classes.
--
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: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
