-
Notifications
You must be signed in to change notification settings - Fork 93
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
pbs: handle poll if PBS client cannot connect #2691
pbs: handle poll if PBS client cannot connect #2691
Conversation
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.
Looks fine. Can this be tested - e.g. by temporarily aliasing the batch sys poll command to a custom or non-existent command?
Yes, thinking about how to write an effective test. |
If PBS qstat cannot connect to its server, assume that jobs managed by it are still OK.
f0d0905
to
fa1c7b6
Compare
Test added. |
@@ -39,6 +39,7 @@ class PBSHandler(object): | |||
# N.B. The "qstat JOB_ID" command returns 1 if JOB_ID is no longer in the | |||
# system, so there is no need to filter its output. | |||
POLL_CMD = "qstat" | |||
POLL_CANT_CONNECT_ERR = "Connection refused" |
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.
How stable is this message across different pbs
versions?
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.
We only have our own to test on our site, so no idea. Our admin team is confident that it is a good message to use though. @hjoliver may have a better insight with his connection with the people who develop PBS.
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.
I'll ping my PBS connections...
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.
(however, we can always adapt to other PBS versions in future releases if need be)
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.
(... pinged)
From a contact at Altair:
|
(The PBS v18.x Qstat new features are pretty exciting, but I don't think they would affect the purpose of this PR.) |
If PBS
qstat
cannot connect to its server, assume that jobs managed by it are still OK.