Conversation
rasmusrygaard
force-pushed
the
dev/rasmus/config_store
branch
from
ce0a0d2 to
6e9afdf
Compare
wiltzius-openai
left a comment
wiltzius-openai
left a comment
There was a problem hiding this comment.
How are we going to handle typing of the config documents? Will there be specific config file loaders that have schemas knowing what fields they expect etc, and we'll hand them a ConfigStore to do the actual byte fetching?
I had originally imagined this interface would have more structured config knowledge in it rather than a generic loader, but if those exist elsewhere and this is only meant to abstract away whether its loaded from filesystem or somewhere else it seems reasonable.
| use crate::ConfigStoreResult; | ||
| use crate::ReadConfigDocumentParams; | ||
|
|
||
| /// Storage-neutral reader for path-addressed config documents. |
There was a problem hiding this comment.
are all implementations going to be path-addressed? are we going to invent some special path namespacing to make them generic (rather than fs paths)?
|
|
||
| /// Byte span for a config document parse error. | ||
| #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] | ||
| pub struct ConfigDocumentErrorSpan { |
There was a problem hiding this comment.
interesting that we need structured byte span ranges for errors, I'd have guessed that could be represented simply in an error string
|
The code parses these values into |
|
Huh -- I had assumed we want to decide value precedence on more of a case by case basis rather than using a general toml merge, but I haven't examined how this works today. Fine w/ me if this is simpler and works. |
|
the other thing that's helpful about the toml merges is that presence can be 1/ not set 2/ set to none/default 3/ set to some value. We mostly have |
2 participants
Add a ConfigStore interface for reading config files. By default we'll back this with a local-only implementation, which I'll add in a follow-up