-
Notifications
You must be signed in to change notification settings - Fork 5.5k
SEP-17: Job acknowledgement events #55820
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
Conversation
re-run pr-opensuse15-py3 |
899bbf4
to
f8be9cf
Compare
@lukasraska Please add your integration tests. I will take a look at making them less flaky. |
f8be9cf
to
c61b127
Compare
@dwoz Thanks for looking into this, I've pushed them and rebased to latest master. From my tries, the It also might be worth noting, before the change in logging, these tests using multithreaded Queue were completely failing on Win and OSX (they worked when print was put strategically, as I mentioned on SaltConf). If you can think of how to make them more successful, |
That's interesting, and hints at a race condition. It may be something else, but |
c61b127
to
da4e825
Compare
da4e825
to
43ba87c
Compare
@lukasraska are you still interested in pushing this over the finish line? |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #55820 +/- ##
==========================================
- Coverage 42.89% 40.60% -2.28%
==========================================
Files 1556 1549 -7
Lines 281049 272241 -8808
Branches 62376 58660 -3716
==========================================
- Hits 120536 110526 -10010
- Misses 146825 149369 +2544
+ Partials 13688 12346 -1342
Flags with carried forward coverage won't be shown. Click here to find out 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.
The tests need to be migrated to pytest
-------------------- | ||
|
||
.. versionadded:: Sodium | ||
|
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.
Chlorine or Argon
Also needs a changelog |
This can be re-based and re-opened if it should get into |
What does this PR do?
Implements new event type
salt/job/<JID>/ack/<MID>
that is fired everytime minion receives job for execution.What issues does this PR fix or reference?
saltstack/salt-enhancement-proposals#23
Tests written?
Yes, but unit tests only. I've prepared integration tests as well, but they are too flaky to be used steadily (they use multithreaded Queue and it doesn't play well with these tests on various platforms).
Commits signed with GPG?
Yes
TODO: