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

Email with custom notifications not working #14839

Merged
merged 1 commit into from
Feb 14, 2024
Merged

Email with custom notifications not working #14839

merged 1 commit into from
Feb 14, 2024

Conversation

dmzoneill
Copy link
Member

SUMMARY

This targets "Email with custom notifications not working"
related #13983

This checks for blank body and add templates errors for the user.

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • API

msg = ''
except (TemplateSyntaxError, UndefinedError, SecurityError) as e:
msg = e # __str__
msg += '\n'.join(traceback.format_exception(None, e, e.__traceback__))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not going to work in the way that you expect.

In [1]: try:
   ...:     raise Exception("foo")
   ...: except Exception as e:
   ...:     msg = e
   ...:     msg += "bar"
   ...: 
---------------------------------------------------------------------------
Exception                                 Traceback (most recent call last)
Input In [1], in <cell line: 1>()
      1 try:
----> 2     raise Exception("foo")
      3 except Exception as e:

Exception: foo

During handling of the above exception, another exception occurred:

TypeError                                 Traceback (most recent call last)
Input In [1], in <cell line: 1>()
      3 except Exception as e:
      4     msg = e
----> 5     msg += "bar"

TypeError: unsupported operand type(s) for +=: 'Exception' and 'str'

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, using += to build up strings is an anti-pattern in Python. I'd instead suggest something like

msg = '\n'.join([
    str(e),
    ...whatever...
])

except (TemplateSyntaxError, UndefinedError, SecurityError):
msg = ''
except (TemplateSyntaxError, UndefinedError, SecurityError) as e:
msg = e # __str__
Copy link
Member

Choose a reason for hiding this comment

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

what does # __str__ do?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing.

Copy link
Contributor

@jbradberry jbradberry left a comment

Choose a reason for hiding this comment

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

The concept seems good, but the exception handling is broken as-is.

@dmzoneill dmzoneill force-pushed the 13983 branch 2 times, most recently from e14b4a3 to f390521 Compare February 14, 2024 14:28
@github-actions github-actions bot added the component:awx_collection issues related to the collection for controlling AWX label Feb 14, 2024
@dmzoneill dmzoneill merged commit 19f80c0 into devel Feb 14, 2024
21 checks passed
@dmzoneill dmzoneill deleted the 13983 branch February 14, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:api component:awx_collection issues related to the collection for controlling AWX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants