add support for prepared query templates #296
Conversation
| @@ -179,7 +181,12 @@ def flush | |||
| }, | |||
| "DNS" => { | |||
| "TTL" => "#{ttl}s" | |||
| }}) | |||
| }, | |||
There was a problem hiding this comment.
Are you sure this should be part of this json should be included no matter what?
What about users of consul before 6.4? What if they don't want to make a template?
There was a problem hiding this comment.
From my testing consul creates the Template hash regardless when a query is created, so sending an empty hash shouldn't affect anything as its not actually a template until Type/Regexp are defined.
That said it will probably fail on < 0.6.4 since it doesn't have templating. I can add some logic into this if you prefer to maintain support for it.
|
|
||
| newparam(:template_regexp) do | ||
| desc 'regexp for template' | ||
| validate do |value| |
There was a problem hiding this comment.
Do you have to have to make this a required argument?
Can you default to nil so that users don't have to provide this all the time?
There was a problem hiding this comment.
Yeah, for sure. I'm somewhat new to ruby types so I'll read the docs a bit more and tweak this.
There was a problem hiding this comment.
Simply defaulting to "nil" or whatever and then not injecting template data will make this "automatically" backwards compatible.
There was a problem hiding this comment.
Setting a default value to nil causes a failure to compile. Set them to some sane string defaults.
adamcstephens
force-pushed
the
query_template
branch
from
ec414d2
to
3551bab
Compare
|
I've updated this to add a template parameter, which allows 0.6.3 to work still since its defaulted to false. |
adamcstephens
force-pushed
the
query_template
branch
from
3551bab
to
41bc005
Compare
|
Any more feedback on this? |
|
Nope! ty. |
add support for prepared query templates
Expanding on #288 and the good work by @djtaylor, add support for prepared query templates to the custom type.