Skip to content

Commit d91ae4c

Browse files
authored
Fix GPU validation authorization and accounting retries (#7732)
* ci: honor Triage role for GPU requests * ci: retry transient Slurm accounting failures
1 parent be4f130 commit d91ae4c

3 files changed

Lines changed: 44 additions & 3 deletions

File tree

.ci/slurm/runner.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1220,7 +1220,7 @@ def github_admit() -> int:
12201220
event_data = json.loads(Path(os.environ["GITHUB_EVENT_PATH"]).read_text(encoding="utf-8"))
12211221
user = event_data["comment"]["user"]["login"]
12221222
permission = _gh("repos/{}/collaborators/{}/permission".format(repository, user))
1223-
if permission.get("permission") not in ("admin", "maintain", "write", "triage"):
1223+
if permission.get("role_name") not in ("admin", "maintain", "write", "triage"):
12241224
raise ValueError("commenter needs Triage permission")
12251225
number = str(event_data["issue"]["number"])
12261226
pull = _gh("repos/{}/pulls/{}".format(repository, number))

.ci/slurm/slurm.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,17 @@ def wait(
7171
raise
7272
time.sleep(self.poll_seconds)
7373

74+
failures = 0
7475
for _ in range(30):
75-
rows = self._accounting(ids)
76+
try:
77+
rows = self._accounting(ids)
78+
failures = 0
79+
except SlurmError:
80+
failures += 1
81+
if failures == 6:
82+
raise
83+
time.sleep(self.poll_seconds)
84+
continue
7685
required = []
7786
for job in jobs:
7887
count = self.jobs[job]

.ci/slurm/test_runner.py

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,18 @@ def test_submit_and_accounting_require_each_array_task(self):
259259
self.assertEqual(states["101_0"], ("COMPLETED", "0:0"))
260260
self.assertEqual(states["101_1"], ("FAILED", "1:0"))
261261

262+
def test_accounting_retries_transient_failure(self):
263+
responses = [
264+
mock.Mock(returncode=0, stdout="", stderr=""),
265+
mock.Mock(returncode=1, stdout="", stderr="Socket timed out"),
266+
mock.Mock(returncode=0, stdout="101|COMPLETED|0:0\n", stderr=""),
267+
]
268+
with mock.patch("slurm.subprocess.run", side_effect=responses):
269+
client = slurm.Slurm(poll_seconds=0)
270+
client.jobs["101"] = None
271+
states = client.wait(("101",))
272+
self.assertEqual(states["101"], ("COMPLETED", "0:0"))
273+
262274
def test_pass_requires_successful_slurm_accounting(self):
263275
config = runner.Config(
264276
runner.Site("Example cluster", "https://cluster.example/", "Computing resources were provided by"),
@@ -808,6 +820,26 @@ def test_mpi_startup_failure_requires_complete_signature(self):
808820

809821

810822
class GitHubTests(unittest.TestCase):
823+
def test_read_user_cannot_trigger_pr_validation(self):
824+
with tempfile.TemporaryDirectory() as directory:
825+
root = Path(directory)
826+
event = root / "event.json"
827+
event.write_text(json.dumps({
828+
"comment": {"user": {"login": "reader"}},
829+
"issue": {"number": 23},
830+
}), encoding="utf-8")
831+
output = root / "output"
832+
environment = {
833+
"GITHUB_EVENT_NAME": "issue_comment", "GITHUB_REPOSITORY": "owner/repo",
834+
"GITHUB_EVENT_PATH": str(event), "GITHUB_OUTPUT": str(output),
835+
}
836+
with mock.patch.dict(os.environ, environment, clear=True), \
837+
mock.patch("runner._gh", return_value={"permission": "read", "role_name": "read"}) as api:
838+
with self.assertRaisesRegex(ValueError, "commenter needs Triage permission"):
839+
runner.github_admit()
840+
api.assert_called_once_with("repos/owner/repo/collaborators/reader/permission")
841+
self.assertFalse(output.exists())
842+
811843
def test_pr_comment_is_created_queued_and_updated_in_place(self):
812844
source_sha = "a" * 40
813845
with tempfile.TemporaryDirectory() as directory:
@@ -819,7 +851,7 @@ def test_pr_comment_is_created_queued_and_updated_in_place(self):
819851
}), encoding="utf-8")
820852
output = root / "output"
821853
admitted = [
822-
{"permission": "triage"},
854+
{"permission": "read", "role_name": "triage"},
823855
{"state": "open", "head": {"repo": {"full_name": "owner/fork"}, "sha": source_sha}},
824856
{"id": 456}, {"id": 123},
825857
]

0 commit comments

Comments
 (0)