-
Notifications
You must be signed in to change notification settings - Fork 784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add global -error-on-missing-key CLI argument #1434
base: main
Are you sure you want to change the base?
Conversation
c5a73b9
to
4ed1b0d
Compare
ddb0559
to
a0bc89d
Compare
Hey @imalsogreg, thanks for the PR. I'm not sure about this as the idea was already turned down for pretty good reasons. I see that this attempts to work around those concerns by having it change the global default instead of being a global override, which makes sense. But I don't think it convinces me. If I were to consider it, I'd want it reworked as I don't like adding arguments to Finalize. That call only deals with making sure things are internally good based on the current config... passing an arg changes the semantics and I already think the config semantics are overly complex. Thinking about it some more I have a few lines of thought...
|
@eikenb Thanks for having a look! What would your preference be, between
Totally understand if you don't want to add complexity to the config story here :) |
A colleague pointed out #1420 which looks pretty similar to this in ways and has me leaning toward your 2nd option (rework w/o Finalize change). You could probably even find some ideas on how to work around the Finalize thing there. I do have to say your 5th option does have a certain appeal. Maybe even changing Really threading through the logic is the hardest part given the current code organization. Whichever does that best would be my first choice... maybe compare what #1420 does vs. converting config.Once from a boolean to a enum-ish type and see which looks cleaner. Thanks. |
d18b2f4
to
ae94024
Compare
@imalsogreg Thanks for the pull request. As a consumer of |
This addresses #1047 by adding a
-error-on-missing-key
CLI argument, which applies to all templates rendered in the currentconsult-template
call, except for those that individually override it the template's configuration.This allows a user to turn on missing-key error checking in situations where it is not convenient to write per-template configs, e.g. when we have many calls to
consul-template
that already use CLI arguments rather than config files to specify template source and target locations.