Skip to content

Commit 5c2cc23

Browse files
committed
Apply lint/format pass to advisor-coverage
- Reflow long lines and multi-line calls to satisfy formatter - Convert dict(...) constructor to {} literal in test config helper - Reorder config import alphabetically; drop stray blank lines - Add noqa: G200 to logfilter HEAD-failure warning - Wrap flaky_trunk argMax(...) verdict expression across lines No behavior change — pure formatting/lint compliance across the advisor_coverage lambda, its tests, and the flaky_trunk queries. Signed-off-by: Jean Schmidt <contato@jschmidt.me>
1 parent c7f01eb commit 5c2cc23

12 files changed

Lines changed: 127 additions & 64 deletions

File tree

aws/lambda/pytorch-advisor-coverage/advisor_coverage/bootstrap.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
import boto3
2323
import github
24-
2524
from pytorch_auto_revert.clickhouse_client_helper import CHCliFactory
2625
from pytorch_auto_revert.github_client_helper import GHClientFactory
2726
from pytorch_auto_revert.utils import RetryWithBackoff
@@ -92,9 +91,7 @@ def _get_secret_from_aws(secret_store_name: str) -> _AWSSecrets:
9291
sys.exit(1)
9392

9493

95-
def _mint_scoped_installation_token(
96-
app_id: str, pem: str, installation_id: int
97-
) -> str:
94+
def _mint_scoped_installation_token(app_id: str, pem: str, installation_id: int) -> str:
9895
"""Mint an installation token scoped to `actions:write` only.
9996
10097
Without token_permissions the mint inherits the App's full permission set

aws/lambda/pytorch-advisor-coverage/advisor_coverage/config.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,7 @@ def _validate(self, *, require_backfill_window: bool = False) -> "CoverageConfig
164164
f"{sorted(ALLOWED_REPOS)} — refusing to dispatch against it"
165165
)
166166
if self.mode not in ("ongoing", "backfill"):
167-
raise ValueError(
168-
f"mode must be 'ongoing' or 'backfill', got {self.mode!r}"
169-
)
167+
raise ValueError(f"mode must be 'ongoing' or 'backfill', got {self.mode!r}")
170168
if require_backfill_window and self.mode == "backfill":
171169
if self.as_of_start is None or self.as_of_end is None:
172170
raise ValueError(

aws/lambda/pytorch-advisor-coverage/advisor_coverage/dispatcher.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,7 @@
3232
log = logging.getLogger(__name__)
3333

3434

35-
def _in_emit_window(
36-
ts: Optional[datetime], start: datetime, stop: datetime
37-
) -> bool:
35+
def _in_emit_window(ts: Optional[datetime], start: datetime, stop: datetime) -> bool:
3836
"""True if `ts` is in [start, stop). Missing timestamps are kept."""
3937
if ts is None:
4038
return True
@@ -251,9 +249,7 @@ def _existing_verdicts(self, reds: List[RedSignal]) -> Set[Tuple[str, str]]:
251249
for attempt in RetryWithBackoff():
252250
with attempt:
253251
res = CHCliFactory().client.query(query, parameters=params)
254-
return {
255-
(str(row[0]).strip(), str(row[1])) for row in res.result_rows
256-
}
252+
return {(str(row[0]).strip(), str(row[1])) for row in res.result_rows}
257253

258254
def _dispatch_one(
259255
self,

aws/lambda/pytorch-advisor-coverage/advisor_coverage/logfilter.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,5 @@ def _log_content_length(job_id: int) -> Optional[int]:
5353
log.debug("[coverage] log HEAD %s → HTTP %s", job_id, e.code)
5454
return None
5555
except (urllib.error.URLError, ValueError, OSError) as e:
56-
log.warning("[coverage] log HEAD %s failed: %s", job_id, e)
56+
log.warning("[coverage] log HEAD %s failed: %s", job_id, e) # noqa: G200
5757
return None

aws/lambda/pytorch-advisor-coverage/advisor_coverage/payload.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@
2222
"at or after the suspect commit (newest first)"
2323
)
2424
_LABEL_SUCCESSFUL = (
25-
"successful: baseline commits where this signal was GREEN "
26-
"before the suspect commit"
25+
"successful: baseline commits where this signal was GREEN before the suspect commit"
2726
)
2827

2928

@@ -33,9 +32,7 @@ def _fmt_ts(dt: Optional[datetime]) -> str:
3332
return dt.strftime("%Y-%m-%d %H:%M:%S UTC")
3433

3534

36-
def _event(
37-
*, status: str, run: JobRun, repo_full_name: str
38-
) -> Dict[str, Any]:
35+
def _event(*, status: str, run: JobRun, repo_full_name: str) -> Dict[str, Any]:
3936
return {
4037
"status": status,
4138
"job_name": run.name,
@@ -48,9 +45,7 @@ def _event(
4845
f"https://github.com/{repo_full_name}/actions/runs/"
4946
f"{run.wf_run_id}/job/{run.job_id}"
5047
),
51-
"log_url": (
52-
f"https://ossci-raw-job-status.s3.amazonaws.com/log/{run.job_id}"
53-
),
48+
"log_url": (f"https://ossci-raw-job-status.s3.amazonaws.com/log/{run.job_id}"),
5449
}
5550

5651

aws/lambda/pytorch-advisor-coverage/advisor_coverage/sql.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
identity the /flaky_trunk page joins on.
1919
"""
2020

21-
2221
_CTE_TRUNK_COMMITS = r"""trunk_commits AS (
2322
SELECT
2423
tupleElement(head_commit, 'id') AS head_sha,

aws/lambda/pytorch-advisor-coverage/advisor_coverage/tests/test_advisor_coverage.py

Lines changed: 99 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414
from advisor_coverage import config as config_mod
1515
from advisor_coverage.backfill import run_backfill
1616
from advisor_coverage.config import (
17+
_parse_workflows,
1718
COVERAGE_SIGNAL_KEY_PREFIX,
1819
CoverageConfig,
1920
HARD_CAP_DISPATCHES,
20-
_parse_workflows,
2121
)
2222
from advisor_coverage.dispatcher import CoverageDispatcher
2323
from advisor_coverage.enumeration import (
@@ -34,14 +34,14 @@ def T(minute: int) -> datetime:
3434

3535

3636
def make_config(**overrides) -> CoverageConfig:
37-
defaults = dict(
38-
repo_full_name="pytorch/pytorch",
39-
workflows=[],
40-
hours=24,
41-
max_dispatches_per_run=10,
42-
dispatch_gap_seconds=3,
43-
dry_run=True,
44-
)
37+
defaults = {
38+
"repo_full_name": "pytorch/pytorch",
39+
"workflows": [],
40+
"hours": 24,
41+
"max_dispatches_per_run": 10,
42+
"dispatch_gap_seconds": 3,
43+
"dry_run": True,
44+
}
4545
defaults.update(overrides)
4646
return CoverageConfig(**defaults)
4747

@@ -96,7 +96,9 @@ def _urlopen_cm(status, content_length):
9696
"""A fake urlopen() context manager for logfilter HEAD tests."""
9797
resp = MagicMock()
9898
resp.status = status
99-
resp.headers = {} if content_length is None else {"Content-Length": str(content_length)}
99+
resp.headers = (
100+
{} if content_length is None else {"Content-Length": str(content_length)}
101+
)
100102
cm = MagicMock()
101103
cm.__enter__.return_value = resp
102104
cm.__exit__.return_value = False
@@ -119,7 +121,9 @@ def __init__(
119121
self.reds = reds
120122
self.config = config or make_config()
121123
self.existing_rows = existing_rows or []
122-
self.limit = limit if limit is not None else self.config.effective_max_dispatches()
124+
self.limit = (
125+
limit if limit is not None else self.config.effective_max_dispatches()
126+
)
123127
self.dispatch_side_effect = dispatch_side_effect
124128
self.sleep_calls = []
125129
self.log_calls = []
@@ -238,11 +242,10 @@ def test_prefix_not_configurable(self):
238242
self.assertFalse(hasattr(cfg, "coverage_prefix"))
239243

240244
def test_dispatch_guard_never_posts_native_key(self):
241-
with _DispatchHarness([make_red()]) as h:
242-
with patch("advisor_coverage.dispatcher.COVERAGE_SIGNAL_KEY_PREFIX", ""), patch(
243-
"advisor_coverage.payload.COVERAGE_SIGNAL_KEY_PREFIX", ""
244-
):
245-
stats = h.dispatcher.dispatch_for_window(T(0), T(59))
245+
with _DispatchHarness([make_red()]) as h, patch(
246+
"advisor_coverage.dispatcher.COVERAGE_SIGNAL_KEY_PREFIX", ""
247+
), patch("advisor_coverage.payload.COVERAGE_SIGNAL_KEY_PREFIX", ""):
248+
stats = h.dispatcher.dispatch_for_window(T(0), T(59))
246249
self.assertEqual(stats.dispatched, 0)
247250
self.assertEqual(stats.errors, 1)
248251
h.dispatch_mock.assert_not_called()
@@ -423,13 +426,17 @@ def test_counters_count_successes_only(self):
423426
class TestConfig(unittest.TestCase):
424427
def test_hard_cap(self):
425428
self.assertEqual(
426-
make_config(max_dispatches_per_run=10000, dispatch_gap_seconds=1).effective_max_dispatches(),
429+
make_config(
430+
max_dispatches_per_run=10000, dispatch_gap_seconds=1
431+
).effective_max_dispatches(),
427432
HARD_CAP_DISPATCHES,
428433
)
429434

430435
def test_timeout_clamp(self):
431436
self.assertEqual(
432-
make_config(max_dispatches_per_run=10000, dispatch_gap_seconds=3).effective_max_dispatches(),
437+
make_config(
438+
max_dispatches_per_run=10000, dispatch_gap_seconds=3
439+
).effective_max_dispatches(),
433440
87,
434441
)
435442

@@ -456,7 +463,12 @@ def test_workflows_env_and_event(self):
456463

457464
def test_lowercase_event_overrides(self):
458465
cfg = CoverageConfig.from_env_and_event(
459-
{"mode": "backfill", "hours": 48, "as_of_start": "2026-01-01", "as_of_end": "2026-01-05"}
466+
{
467+
"mode": "backfill",
468+
"hours": 48,
469+
"as_of_start": "2026-01-01",
470+
"as_of_end": "2026-01-05",
471+
}
460472
)
461473
self.assertEqual(cfg.mode, "backfill")
462474
self.assertEqual(cfg.hours, 48)
@@ -480,7 +492,11 @@ def test_backfill_requires_as_of_window(self):
480492
def test_backfill_as_of_must_be_ordered(self):
481493
with self.assertRaises(ValueError):
482494
CoverageConfig.from_env_and_event(
483-
{"mode": "backfill", "as_of_start": "2026-01-05", "as_of_end": "2026-01-01"}
495+
{
496+
"mode": "backfill",
497+
"as_of_start": "2026-01-05",
498+
"as_of_end": "2026-01-01",
499+
}
484500
)
485501

486502

@@ -496,22 +512,68 @@ def _result(self, columns, rows):
496512

497513
def test_assembles_red_with_baselines_before(self):
498514
unc_cols = [
499-
"head_sha", "commit_time", "workflow_name", "name", "cons_name",
500-
"job_id", "run_id", "run_attempt", "started_at", "completed_at",
515+
"head_sha",
516+
"commit_time",
517+
"workflow_name",
518+
"name",
519+
"cons_name",
520+
"job_id",
521+
"run_id",
522+
"run_attempt",
523+
"started_at",
524+
"completed_at",
501525
]
502526
unc_rows = [
503-
("sha_obs", T(30), "slow", "linux / test (slow, 2, 3, runner-a)",
504-
"linux / test (slow, 2, 3)", 900, 901, 1, T(30), T(40)),
527+
(
528+
"sha_obs",
529+
T(30),
530+
"slow",
531+
"linux / test (slow, 2, 3, runner-a)",
532+
"linux / test (slow, 2, 3)",
533+
900,
534+
901,
535+
1,
536+
T(30),
537+
T(40),
538+
),
505539
]
506540
base_cols = [
507-
"workflow_name", "cons_name", "head_sha", "commit_time", "name",
508-
"job_id", "run_id", "run_attempt", "started_at", "completed_at",
541+
"workflow_name",
542+
"cons_name",
543+
"head_sha",
544+
"commit_time",
545+
"name",
546+
"job_id",
547+
"run_id",
548+
"run_attempt",
549+
"started_at",
550+
"completed_at",
509551
]
510552
base_rows = [
511-
("slow", "linux / test (slow, 2, 3)", "sha_future", T(50),
512-
"linux / test (slow, 2, 3, runner-a)", 700, 701, 1, T(50), T(55)),
513-
("slow", "linux / test (slow, 2, 3)", "sha_before", T(20),
514-
"linux / test (slow, 2, 3, runner-a)", 800, 801, 1, T(20), T(25)),
553+
(
554+
"slow",
555+
"linux / test (slow, 2, 3)",
556+
"sha_future",
557+
T(50),
558+
"linux / test (slow, 2, 3, runner-a)",
559+
700,
560+
701,
561+
1,
562+
T(50),
563+
T(55),
564+
),
565+
(
566+
"slow",
567+
"linux / test (slow, 2, 3)",
568+
"sha_before",
569+
T(20),
570+
"linux / test (slow, 2, 3, runner-a)",
571+
800,
572+
801,
573+
1,
574+
T(20),
575+
T(25),
576+
),
515577
]
516578
mock_ch = MagicMock()
517579
mock_ch.return_value.client.query.side_effect = [
@@ -553,8 +615,12 @@ def test_workflow_filter_applied_only_when_set(self):
553615
enum_wf = UnclassifiedRedEnumerator(make_config(workflows=["slow"]))
554616
from advisor_coverage.sql import QUERY_UNCLASSIFIED
555617

556-
self.assertNotIn("workflow_name IN", enum_all._apply_workflow_filter(QUERY_UNCLASSIFIED))
557-
self.assertIn("workflow_name IN", enum_wf._apply_workflow_filter(QUERY_UNCLASSIFIED))
618+
self.assertNotIn(
619+
"workflow_name IN", enum_all._apply_workflow_filter(QUERY_UNCLASSIFIED)
620+
)
621+
self.assertIn(
622+
"workflow_name IN", enum_wf._apply_workflow_filter(QUERY_UNCLASSIFIED)
623+
)
558624

559625

560626
# ----------------------------------------------------------------------
@@ -639,9 +705,7 @@ def test_cursor_roundtrips_through_parse_datetime(self):
639705
parsed = CoverageConfig.from_env_and_event(
640706
{"mode": "backfill", "as_of_start": cursor, "as_of_end": "2026-01-02"}
641707
)
642-
self.assertEqual(
643-
parsed.as_of_start.strftime("%Y-%m-%d %H:%M:%S"), cursor
644-
)
708+
self.assertEqual(parsed.as_of_start.strftime("%Y-%m-%d %H:%M:%S"), cursor)
645709

646710

647711
# ----------------------------------------------------------------------

aws/lambda/pytorch-advisor-coverage/advisor_coverage/tests/test_live.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,10 @@ def test_enumeration_runs_no_illegal_aggregation(self):
5959
print(f"\n[live] unclassified reds in last 48h: {len(reds)}")
6060
if reds:
6161
total_baselines = sum(len(r.baselines) for r in reds)
62-
print(f"[live] sample suspect job_id={reds[0].suspect.job_id} "
63-
f"key={reds[0].job_name!r} baselines_total={total_baselines}")
62+
print(
63+
f"[live] sample suspect job_id={reds[0].suspect.job_id} "
64+
f"key={reds[0].job_name!r} baselines_total={total_baselines}"
65+
)
6466

6567

6668
@unittest.skipUnless(_LIVE, "needs COVERAGE_LIVE_TESTS=1 (network)")

torchci/clickhouse_queries/flaky_trunk_entity_runs/query.sql

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,10 @@ advisor_agg AS (
6464
) AS adv_norm,
6565
-- Prefer a native (non-coverage_) verdict over a coverage_ one for the same
6666
-- (commit, job); among equals, the latest timestamp wins.
67-
argMax(verdict, (toUInt8(NOT startsWith(signal_key, 'coverage_')), timestamp)) AS verdict
67+
argMax(
68+
verdict,
69+
(toUInt8(NOT startsWith(signal_key, 'coverage_')), timestamp)
70+
) AS verdict
6871
FROM misc.autorevert_advisor_verdicts
6972
WHERE
7073
repo = {repo: String}

torchci/clickhouse_queries/flaky_trunk_jobs/query.sql

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,10 @@ advisor_agg AS (
6969
) AS adv_norm,
7070
-- Prefer a native (non-coverage_) verdict over a coverage_ one for the same
7171
-- (commit, job); among equals, the latest timestamp wins.
72-
argMax(verdict, (toUInt8(NOT startsWith(signal_key, 'coverage_')), timestamp)) AS verdict
72+
argMax(
73+
verdict,
74+
(toUInt8(NOT startsWith(signal_key, 'coverage_')), timestamp)
75+
) AS verdict
7376
FROM misc.autorevert_advisor_verdicts
7477
WHERE
7578
repo = {repo: String}

0 commit comments

Comments
 (0)