Open vasily-kirichenko opened 9 years ago
I was just looking into this (because I'd like to use YamlConfig for a project) and noticed this can be worked around by immediately calling s.Load
and passing the same path that was given to the static parameter. Doesn't fix the issue, but at least the list can still be loaded correctly at runtime. :)
the file may not be available at runtime.
If you distribute it with your binary, it will be. :wink:
Like I said, doesn't fix the issue. In a few past projects I've looked at YamlConfig and given up on it due to this issue, so I figured posting a workaround would be worth it in case anyone else runs into this and really wants to use YamlConfig in the meantime. I'm used to distributing app.config anyway, so this isn't really any worse.
WRT fixing it, I've been poking around a bit and will open a PR if I get anywhere useful. While I'm commenting, if each list element contains different values internally, it seems like this should be updated to detect all of the fields instead of just the fields in the first list element. May be a good idea to then also warn/error if a field appears to have different types under different list elements?
I've got a small change done over at my fork, but I'm not sure where to go with initializing the values for each list element. It seems like a good answer would be for listCtr
to create all of the elements and then go call all of the setters, but I'll admit I haven't wrapped my head around everything that's going on.
Just embedding the source yaml as a constant and calling LoadText
in Root
's ctor would solve all of this for my needs, but it's a very blunt option and doesn't fix the issue so much as just cover it up.
Oh dear, this is still an issue? Ran into it. @amazingant, maybe it is a better idea to look at the list and to use the union of all fields for the data-type, and perhaps mark some mandatory and others optional, based on their occurences. The JSon and CSV type providers both generate types based on example data, and later you read in the real data. I was trying to use yaml as a data-medium and not a configuration by the way, not sure if that is a good idea yet.
@fjoppe the change I linked above adds the first part of what you suggested -- it collects the union of all detected fields. I didn't get around to marking any fields as optional, but it shouldn't be hard to do?
That said, the speed of feedback here versus my project due dates has left me just making custom builds for $employer with my patches. If doing the same is an acceptable option for you, I've got a fix here in case you try putting a valid int32 into a field that was originally detected as an int64 (throws an exception in the official code), as well as support for YAML's "timestamp" field here (maps to .NET's DateTimeOffset
).
Just to make @amazingant's point a little clearer, this is the type of thing that you're supposed to do:
type AppConfig = YamlConfig<"AppConfig.yaml">
let appConfig = AppConfig()
// The next line assumes you marked AppConfig.yaml as an embedded resource + copy if newer
appConfig.Load("AppConfig.yaml")
There is a comment explaining the situation here https://github.com/fsprojects/FSharp.Configuration/blob/2ac7103314b5b01fe5cd9565feb18d69491dc115/src/FSharp.Configuration.DesignTime/YamlConfigProvider.fs#L111
This bug comes from https://github.com/fsprojects/FSharp.Configuration/issues/51
As I'm new to F# I don't think I'll have a try at coming up with a fix just yet. :)
s.AList
property contains the first element only.