-
Notifications
You must be signed in to change notification settings - Fork 14
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
Check destinations before forking #616
Conversation
Codecov Report
@@ Coverage Diff @@
## master #616 +/- ##
==========================================
+ Coverage 82.04% 82.08% +0.04%
==========================================
Files 73 73
Lines 3531 3539 +8
==========================================
+ Hits 2897 2905 +8
Misses 634 634
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With #618 applied, this works. It's still not superfast, but that's mostly the fault of the bulk endpoint itself...
dc4e344
to
71b5f0d
Compare
71b5f0d
to
aca690f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You marked my suggestion in the find destination test as resolved, but without any comment or changes
It seems my commit via github didn't take, seems to have worked when I tried just now. You might not be allowed to review anymore... |
Co-authored-by: Johanna England <[email protected]>
49c2f13
to
6c1b8cd
Compare
Kudos, SonarCloud Quality Gate passed!
|
The code now only forks a new process if there is anything to send.
Note: The functions in
argus/notificationprofile/media/__init__.py
has been sorted differently since the actual sending functions are parameters to the filter-function and therefore must come before it.Other problems I found:
plugin.send
should be on the base-class and not overriden. Then the base class controls whether it wants a queryset or a list of destinations.