Repository navigation
Add Config to FlagConfig struct - #332
peppi-lotta wants to merge 3 commits into
Conversation
|
@SuperQ Here's my proposed change to enhance Let me know if this aligns with your vision for the refactor. I'm happy to adjust the implementation in any way if needed. |
|
@SuperQ can we have some review on this? It would be good to get this one rolling |
|
Thanks for the PR! As right now this is either config struct or file based, do we see a use case where an app wants to set certain defaults via the config struct and have the user overwrite it via the file? |
|
I think the either config or file is fine. We don't need a combo. |
|
@mrueg I have now added a function for validating the config. Could you please take a look and provide a review when you have a chance? |
|
@mrueg Could we get a review on this? |
5560ca0 to
3c64144
Compare
3c64144 to
583cf60
Compare
|
@ArthurSens @mrueg @SuperQ This has been on hiatus for a while but I feel this is still a good addition. PTAL :) |
|
/approve-workflow |
ArthurSens
left a comment
There was a problem hiding this comment.
I'm not sure if we want WebConfig to override WebConfigFile. I'm happy for them to be mutually exclusive, then downstream projects using exporter-toolkit can implement the overriding logic by themselves. My worry is that today WebConfigFile is the only way of doing this, if the new WebConfig overrides the current behavior, that's a breaking behavior. Feels safer to not do the override 🤔
This allows the Serve-function to be called with out having a config file. This makes the function more easilly callable from other projects. This way configuration is not limited to an existing yaml file but can be specified in the project that is using this package. Signed-off-by: peppi-lotta <peppi-lotta.saari@est.tech>
Signed-off-by: peppi-lotta <peppi-lotta.saari@est.tech>
Signed-off-by: peppi-lotta <peppi-lotta.saari@est.tech>
45fa521 to
c161759
Compare
I've now made WebConfig to override WebConfigFile mutually exclusive. If both are set there will be a "conflicting flags" error. I've added necessary unit testing for this as well. |
This refactoring allows the
Servefunction to be called without requiring a YAML configuration file. As a result, the function becomes easier to use from other projects, since configuration is no longer tied to an external YAML file.The change was proposed in PR #7255 to the CoreDNS project. To maintain backward compatibility while enabling this flexibility, I added a
Configfield to theFlagConfigstruct. This approach preserves the existing behavior while allowing external projects to inject configuration directly.