Open Horusiath opened 6 years ago
I wonder if an interface based approach might not be more flexible. The basic idea is that you define an interface, point it to a Config
, and an instance of a dynamically generated immutable type that implements the interface is generated.
The basic implementation would leverage the field types currently in Config
, but be extensible by registering parsers, understanding collections and reusing previously defined interfaces. Mappings can be tailored with attributes to rename, select defaults, and redirection to other pieces of HOCON.
public interface IMySettings
{
int MyFirstField { get; }
[Default("If missing use this value")]
string MySecondField { get; }
[Hocon(Name = "map-from-this-name")]
string MyThirdField { get; }
[Hocon(Path = "get-values.from-here")]
IList<IPAddress> Addresses { get; }
IYourSettings Options1 { get; }
}
var settings = system.Settings.Config
.GetConfig("akka.my.settings")
.Get<IMySettings>();
This would map onto a HOCON like this:
akka {
my {
settings {
MyFirstField = 42
map-from-this-name = Hello
get-values {
from-here = ['127.0.0.1', '196.0.0.10']
}
Options1 {
field1 = 11
field2 = woo-hoo
}
this-value-is-not-mapped = nope
}
}
}
If system.Settings.Config.GetConfig("akka.my.settings")
is null
the values can be loaded entirely from the attributes on the interface, assuming the interface is sufficiently attributed.
I currently have an implementation that is very like this, except that it works an abstract tree rather than directly with HOCON.
@pshrosbree what is the value added of the interface here? We'd need to create dynamic proxies for a concrete implementation. Also with your approach I don't really see any place where validation logic could be placed.
Off the top of my head, it avoids the looking up by constructor signature, which I have found messy and confusing, allows the injection of values into instances of types that implement the interface, makes attributing cleaner, and missing values can be more easily defaulted, making matching simpler.
Yes, dynamically creating types and creating an instance is the typical scenario, but I have written that already. However, it is not necessary, as you could provide an instance of a type that implements the interface:
var settings = system.Settings.Config
.GetConfig("akka.my.settings")
.Get<IMySettings>(instance);
Doing anything in constructors besides assigning instance variables is something you want to avoid in OO, so validation in the constructor would be an anti-pattern, and library users would have to implement that anti-pattern. However, we could come up with a delegate based static factory method for avoiding that anti-pattern.
The attributing I mention in my previous post would apply to both cases, as would the extensible parsing mechanism, but it would be syntactically cleaner and more succinct in the interface case.
Consider the above mapping using a constructor:
public class MySettings
{
public MySettings(
int MyFirstField,
[Default("If missing use this value")]
string MySecondField,
[Hocon(Name = "map-from-this-name")]
string MyThirdField,
[Hocon(Path = "get-values.from-here")]
IList<IPAddress> Addresses,
IYourSettings Options1)
{
myFirstField = MyFirstField;
mySecondField = MySecondField;
myThirdField = MyThirdField;
addresses = Addresses;
options1 = Options1;
}
int myFirstField;
string mySecondField;
string myThirdField;
IList<IPAddress> addresses;
IYourSettings options1;
}
Here you see that in C# we'd have to write properties three times in the constructor case (fours times in the delegate factory case), but only once in the interface case.
Inter-field validation is tricky in all cases for configuration, and I don't see a good declarative way around that.
This is YAGNI, IMHO. Unless you're writing your own custom HOCON values and consuming those inside custom settings classes, I don't see any value in adding this to the framework.
Sorry, my last reply was too dismissive - here's what I should have said:
"I'm not convinced the juice is worth the squeeze here, but I'd love to hear some arguments why we should add this. Right now I view this as just another thing that we'd need to QA, maintain, and debug and not something that would add a lot of value to the framework.
However, if the goal is to make it easier to integrate HOCON into other users' settings classes and start encouraging this as a good practice, rather than this being something for internal use, that's a different story. I'd like to hear more about that?"
Given that, could you guys help me change my mind?
I imagined this as being in another package, and not the framework, partly because it is a bit of a departure from the Lightbend stuff.
However, having an extensible type validated configuration system based on HOCON is useful.
For instance, I store the name of a secret from Key Vault in HOCON, and the interface member is, for instance, a Secret<CloudStorageAccount>
, which can update the secret when it is rotated, and all I have to do to enable this is write the secret name in the HOCON.
I'll try to put something up on GitHub in the next few weeks.
@pshrosbree I'd be up for putting this as an additional package in the HOCON repository, since we're trying to move most of that stuff out of this repo in the first place.
Among other reasons, to allow for more changes like this to be made without affecting the "big infrastructure" parts of the core actor library itself.
@pshrosbree @Aaronontheweb My rationale for this is our current approach visible in many of different configs. This is typical pattern for building typed config objects:
// this is how most C# people writte configs
public class MySettings
{
public string X { get; set; }
public int Y { get; set; }
}
However in Akka.NET we use immutable configuration with copying mutations and params verification:
public sealed class MySettings
{
public string X { get; }
public int Y { get: }
public MySettings(string x, int y)
{
// optional place for checks and validation
if (y <= 0) throw new ArgumentException("y must be greater than 0");
X = x;
Y = y;
}
public MySettings(Config config) : this(x: config.GetString("x"), y: config.GetInt("y")) {}
public MySettings WithX(string x) => new MySettings(x, this.Y);
public MySettings WithY(int y) => new MySettings(this.X, y);
}
You can find code like that in dozen of places often with 20+ parameters (I know cause I've wrote it): while other parts can be automized by tooling (like R# or Fody With
plugin), mapping values from Config to object is always manual labor and it's error prone (typos).
Optionally the second (immutable) case could be also written in F# in following manner:
type MySettings =
{ X: string; Y: int }
// again we need to write mapping logic.
static member FromConfig(config: Config) =
{ X = config.GetString("x");
Y = config.GetInt("y") }
What I want to do here is an ability to just say config.Get<MySettings>()
that would resolve config values written in all 3 cases here. This already has been done by Microsoft.Extensions.Configuration, however in very poor version (only 1st case supported).
In most cases, when we are creating a HOCON configuration, internally we are still mapping it on immutable settings objects. In that case mapping HOCON key=value pairs to class properties is a manual work... and a lot of boilerplate.
The idea here is to provide some sort of method on top of
Config
class, that will be able to build desired object automatically, i.e:The idea for the algorithm:
[HoconConstructor]
.This way we could use constructor for our arbitrary validation logic. While not every settings type we have in our code, would match this approach one to one, I think it's also a good way to standardize our approach to configuration.