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

Various SMTP returner fixes #31688

Merged
merged 6 commits into from Mar 9, 2016
Merged

Various SMTP returner fixes #31688

merged 6 commits into from Mar 9, 2016

Conversation

whiteinge
Copy link
Contributor

What does this PR do?

SMTP returner breakage.

What issues does this PR fix or reference?

The renderer & template additions to the SMTP returner are not working with default settings. The resulting email was blank and the Subject line was <StringIO object>. Switching to only the 'jinja' renderer produces <StringIO object> for both the body and the subject.

Previous Behavior

Not working with default settings.

New Behavior

Backward-compatible fixes assuming the renderer & template additions were indeed broken as previously asserted.

Tests written?

  • Yes
  • No
  • Not sure

YAML doesn't make sense here since we're just rendering a text email.
The user can still override in the smtp renderer settings if the user
has complex needs.
compile_template returns StringIO objects.
This is a frequent stumbling block for users unaccustomed to the older
`mod.setting` configuration syntax. Often they will interpret the list
of settings as needing to be underneat a top-level `smtp` dictionary.

This has been on my todo list for a looong time. :-P
@whiteinge
Copy link
Contributor Author

I would appreciate a little assistance on this, if someone has time. I didn't dive back to see if they were broken recently or broken on arrival. There are tests for this module but I didn't look at them to see what they missed.

@whiteinge
Copy link
Contributor Author

Re: the test failure in Jenkins. compile_template can return a variety of things depending on the renderer(s) used, which makes sense. The email body & subject need to be strings. How to enforce?

@rallytime
Copy link
Contributor

@jfindlay Can you take a look here when you get a moment?

@whiteinge
Copy link
Contributor Author

I found the subject setting was also messed up.

Based on @rallytime's feedback I added a commit to explicitly check for the StringIO object that Jinja returns. This should be more error-tolerant.

@whiteinge
Copy link
Contributor Author

Go Go Jenkins!

2 similar comments
@whiteinge
Copy link
Contributor Author

Go Go Jenkins!

@rallytime
Copy link
Contributor

Go Go Jenkins!

rallytime pushed a commit that referenced this pull request Mar 9, 2016
@rallytime rallytime merged commit 286ea1f into saltstack:2015.8 Mar 9, 2016
@whiteinge whiteinge deleted the smtp-renderer branch March 10, 2016 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants