Skip to content
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

pass without exception if jinja parameter isn't found #6101

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

guzzijones
Copy link
Contributor

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:

{ "test": "{{some_text_here}}"}

These cause stackstorm to attempt to resolve some_text_here as a variable since it has matching {{ and }}.

This pr would allow those through as normal strings if they cannot be resolved.

@pull-request-size pull-request-size bot added the size/M PR that changes 30-99 lines. Good size to review. label Dec 21, 2023
@CLAassistant
Copy link

CLAassistant commented Dec 21, 2023

CLA assistant check
All committers have signed the CLA.

@arm4b

This comment was marked as off-topic.

Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@guzzijones
Copy link
Contributor Author

I don't understand:

collect a list of nodes (vars) that aren't valid as template vars

@guzzijones
Copy link
Contributor Author

guzzijones commented Feb 24, 2024

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 {{...}} in the json as a string. These are unable to be processed by Stackstorm currently.
I actually don't understand the use case for putting {{...}} templating parameter evaluation inside stackstorm except for the case of config context or secrets from the KV store.

One can always just utilize a parameter in their python script.

@nzlosh
Copy link
Contributor

nzlosh commented Feb 24, 2024

I'd rather see a more deterministic method to processing (or not) template strings. For example, adding a new input data type raw_string, which will not attempt to process the input variable and treat it at as raw string until it is handed off to the workflow/action. string would remain unchanged and process strings against template detection.

@pull-request-size pull-request-size bot added size/L PR that changes 100-499 lines. Requires some effort to review. and removed size/M PR that changes 30-99 lines. Good size to review. labels Mar 8, 2024
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
st2common/st2common/util/param.py Outdated Show resolved Hide resolved
st2common/st2common/util/param.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L PR that changes 100-499 lines. Requires some effort to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants