pass without exception if jinja parameter isn't found #6101
Conversation
pull-request-size
bot
added
the
size/M
|
|
guzzijones
force-pushed
the
upstream_embedded_jinja
branch
from
49280d1
to
9f8c861
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this comment.
Maybe we could:
- collect a list of nodes (vars) that aren't valid as template vars
- if ANY of the vars DO exist, throw an error for the missing nodes.
- if NONE of the vars exist, then assume it is a string and ignore.
Otherwise, I'm concerned this could hide misspellings.
|
I don't understand:
|
|
Most, if not all, my actions already have a jinja template parameter for {{config_context.var}}. So I don't think check if ALL are parameters would work in my case. We have a lot of log files with One can always just utilize a parameter in their python script. |
|
I'd rather see a more deterministic method to processing (or not) template strings. For example, adding a new input data type |
pull-request-size
bot
added
size/L
fix tests so they do not check for exception for rendering parameters remove debugging and clean up comments remove comments in params rendering code black fixes st2common tests remove unused var
guzzijones
force-pushed
the
upstream_embedded_jinja
branch
from
e795566
to
9f46039
Compare
does not raise and error any more
|
It would be nice to get this merged in. We have a lot of logs with |
|
Can we get some eyes on this? |
| check_any_bad.append(True) | ||
| if name not in tracked_parents: | ||
| children = [i for i in G.predecessors(name)] | ||
| tracked_parents.extend(nodes) |
There was a problem hiding this comment.
| tracked_parents.extend(nodes) | |
| tracked_parents.append(name) |
With extend, only the first child gets checked, not any of the others. Is that intentional? If so, it's not clear why we want to do that.
| for i in nodes: | ||
| if "template" in g_copy.nodes[parent].keys(): | ||
| g_copy.nodes[parent]["value"] = g_copy.nodes[parent].pop("template") | ||
| if i in g_copy.nodes: |
There was a problem hiding this comment.
i is reused here a couple of times. Could you rename this one to something like node or name to make its purpose clearer?
| children = [i for i in g_copy.predecessors(i)] | ||
| if children: | ||
| if i not in tracked_parents: | ||
| _remove_bad(g_copy, i, children) |
There was a problem hiding this comment.
Does this line need to pass in tracked_parents? If not, why not?
| if i in g_copy.nodes: | ||
| children = [i for i in g_copy.predecessors(i)] | ||
| if children: | ||
| if i not in tracked_parents: |
There was a problem hiding this comment.
I don't see anywhere that adds the children to tracked_parents while walking the tree which means the if i not in tracked_parents: condition will always be true.
If a jinja parameter doesn't exist as an input parameter then do not throw an exception and leave the value as a string parameter.
We have some input paramters like:
These cause stackstorm to attempt to resolve
some_text_hereas a variable since it has matching {{ and }}.This pr would allow those through as normal strings if they cannot be resolved.