PP-374: PTL for Job with a root-owned script runs even if root_reject… #232
Conversation
|
Thanks @hirenvadalia for reviewing the changes. I've addressed your comments (from #231 ) in this pull request. |
| rv = self.server.expect(JOB, {'job_state': 'Q'}, id=jid) | ||
| self.assertTrue(rv) | ||
| _comment = 'Not Running: PBS Error: Execution server rejected request' | ||
| rv = self.server.expect(JOB, {'comment': _comment}, id=jid) |
There was a problem hiding this comment.
You don't need to check the return code from expect(). If it isn't true, it'll error out. The only reason you'll reach your assertTrue() is if it is true.
| submit a script as root and observe the job comment. | ||
| """ | ||
| j = Job(ROOT_USER) | ||
| j.create_script(self.sleep_5) |
There was a problem hiding this comment.
Idle curiosity: Why are you using a job script instead of the default job? You don't depend on anything the script is doing.
There was a problem hiding this comment.
Only a job script will get rejected by the mom, not a default job if the "reject_root_scripts" parameter is set to true.
| j.create_script(self.sleep_5) | ||
| jid = self.server.submit(j) | ||
| rv = self.server.expect( | ||
| JOB, {'job_state': 'R'}, id=jid) |
There was a problem hiding this comment.
No need to break this line. The two lines together is 63 characters. Please join the two.
| j.create_script(self.sleep_5) | ||
| jid = self.server.submit(j) | ||
| rv = self.server.expect( | ||
| JOB, {'job_state': 'R'}, id=jid) |
There was a problem hiding this comment.
Same as above about checking the return value of expect(). It'll error out if the expect fails.
|
Hello @bhroam , |
|
Looks good to me. I sign off. |
…_scripts is set to true
|
I've squashed and rebased my commits. Please take it further. |
…_scripts is set to true
Issue-ID
Problem description
Cause / Analysis
Solution description
Checklist:
For further information please visit the Developer Guide Home.