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.

Sign up for GitHub

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

Jump to bottom

Respect argline passed into jinja renderer #55126

Merged
merged 8 commits into from
Apr 12, 2020
Merged

Respect argline passed into jinja renderer #55126

merged 8 commits into from
Apr 12, 2020

Conversation

terminalmage
Copy link
Contributor

Resolves #55124.

@terminalmage terminalmage requested a review from a team as a code owner October 25, 2019 17:15
@ghost ghost requested a review from waynew October 25, 2019 17:15
@terminalmage terminalmage mentioned this pull request Oct 25, 2019
Closed
dwoz
dwoz previously approved these changes Oct 28, 2019
View reviewed changes
@terminalmage
Copy link
Contributor Author

I just realized that I opened this PR against master. The goal is to have this fix in 2019.2.3, does it need to be rebased against a different branch for this?

@Ch3LL Ch3LL added the v2019.2.3 unsupported version label Nov 7, 2019
@Ch3LL Ch3LL mentioned this pull request Dec 3, 2019
Closed
@waynew
Copy link
Contributor

waynew commented Dec 27, 2019

Looks like there are some possibly related test failures?

Traceback (most recent call last):
  File "/tmp/kitchen/testing/tests/unit/utils/test_jinja.py", line 1492, in test_normlookup
    ret = self.render(tmpl_str)
  File "/tmp/kitchen/testing/tests/unit/utils/test_jinja.py", line 1483, in render
    return self.jinja(tmpl_str, context=context or {}, from_str=True).read()
  File "/tmp/kitchen/testing/salt/renderers/jinja.py", line 71, in render
    **kws)
TypeError: render_tmpl() got multiple values for keyword argument 'from_str'

@terminalmage
Copy link
Contributor Author

The tests that were failing were failing because they were not designed properly. from_str is supposed to be derived from the argline, not passed in via the double-asterisk catch-all. I fixed the failing tests.

@dwoz
Copy link
Contributor

dwoz commented Jan 4, 2020

@terminalmage @waynew the windows failures look legit.

@terminalmage
Copy link
Contributor Author

@dwoz I pushed an update to the tests, and rebased to bring it up to date with master.

@sagetherage sagetherage removed the v2019.2.3 unsupported version label Mar 19, 2020
@sagetherage
Copy link
Contributor

@terminalmage looks like there are pending conflicts when you get time :) I'm updating the labels to next release so that it isn't confusing

@sagetherage sagetherage added the ZRelease-Sodium retired label label Mar 19, 2020
@terminalmage
Copy link
Contributor Author

Rebased again.

Erik Johnson added 4 commits April 6, 2020 21:52
@terminalmage
Respect argline passed into jinja renderer
275083f 
This was being ignored previously, despite being checked for in the
beginning of the render() function. This caused any renderer passing
string data, as opposed to a StringIO or other file-like object (e.g.
the gpg renderer) to fail jinja rendering, as the salt.utils.templates
was not being properly informed that the input was a string.
@terminalmage
Add test case for saltstack#55124
3970a93
@terminalmage
Fix incorrectly-written call to jinja renderer in unit tests
9dcc1a2
@terminalmage
Use a command that also works on Windows
594a658
@terminalmage
Copy link
Contributor Author

Rebased yet again. It has been 5 1/2 months since this PR was opened. It's a one-line code change, and was submitted with tests.

Can we please get this re-re-re-reviewed and merged?

@dwoz dwoz merged commit 150f2d9 into saltstack:master Apr 12, 2020
@terminalmage terminalmage deleted the issue55124 branch February 13, 2024 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@waynew waynew waynew requested changes

@DmitryKuzmenko DmitryKuzmenko DmitryKuzmenko approved these changes

@dwoz dwoz dwoz approved these changes

Assignees
No one assigned
Labels
ZRelease-Sodium retired label
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Jinja renderer ignores argline
6 participants
@terminalmage @waynew @dwoz @sagetherage @DmitryKuzmenko @Ch3LL