Skip to content

Update mailer.py with some improvements#415

Open
ShadowJonathan wants to merge 2 commits into
alerta:masterfrom
ShadowJonathan:remove-autoescape
Open

Update mailer.py with some improvements#415
ShadowJonathan wants to merge 2 commits into
alerta:masterfrom
ShadowJonathan:remove-autoescape

Conversation

@ShadowJonathan

Copy link
Copy Markdown

Description

Fixes #393, making sure the mailer works

Changes

  • Remove 2 print statements of exceptions that prevented me from finding this bug in the first place; just let python print the traceback
  • Remove one dependency statement for jinja2, it is now built-in

Checklist

  • Pull request is limited to a single purpose
  • Code style/formatting is consistent
  • All existing tests are passing
  • Added new tests related to change
  • No unnecessary whitespace changes

Collaboration
When a user creates a pull request from a fork that they own, the user
generally has the authority to decide if other users can commit to the
pull request's compare branch. If the pull request author wants greater
collaboration, they can grant maintainers of the upstream repository
(that is, anyone with push access to the upstream repository) permission
to commit to the pull request's compare branch

See https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

@satterly satterly left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the improvements! A few issues need addressing before this can be merged:

  1. Bug: stale variable referencex.strip() should be c.strip() (or just c, since rule_contacts values are already stripped). The old list comprehension variable x no longer exists, so this will raise a NameError at runtime whenever a matching non-exclude rule is found.

  2. break on exclude match changes behavior — The new break after an exclude rule means only the first matching exclude rule applies and no further rules are evaluated. Is this intentional? If so, please document the rationale.

  3. Main-thread exception handling removed — Removing the except Exception blocks in main() means main-thread exceptions now produce a full traceback to stderr instead of a clean message + exit. This is a behavior change worth noting.

Please fix #1 at minimum — it's a runtime crash.

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.

[Alerta-Mailer] Integration failed with Jinja2 Autoescape extension deprecation

2 participants