Skip to content

Commit

Permalink
Remove now-obsolete fine-grained calculation of metrics for archived …
Browse files Browse the repository at this point in the history
…repos

See last commit for context.
  • Loading branch information
benbc committed Mar 13, 2024
1 parent 686a7d1 commit 4c6c4d5
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 63 deletions.
5 changes: 1 addition & 4 deletions metrics/github/prs.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,7 @@ def calculate_counts(prs_by_repo, predicate):
for repo, prs in prs_by_repo.items():
for pr in prs:
start = pr["created_on"]
end = min(
repo.archived_on if repo.archived_on else date.today(),
pr["closed_on"] if pr["closed_on"] else date.today(),
)
end = pr["closed_on"] if pr["closed_on"] else date.today()
for day in iter_days(start, end):
if predicate(pr, day):
counts[(repo.org, repo.name, pr["author"], day)] += 1
Expand Down
7 changes: 2 additions & 5 deletions metrics/github/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def repos(client, org):
org,
raw_repo["name"],
date_from_iso(raw_repo["createdAt"]),
date_from_iso(raw_repo["archivedAt"]),
raw_repo["archivedAt"] is not None,
raw_repo["hasVulnerabilityAlertsEnabled"],
)

Expand All @@ -42,12 +42,9 @@ class Repo:
org: str
name: str
created_on: date
archived_on: date | None
is_archived: bool = False
has_vulnerability_alerts_enabled: bool = False

def is_archived(self):
return bool(self.archived_on)


def team_repos(client, org, team):
"""The API doesn't make it easy for us to get all the information we need about repos in
Expand Down
2 changes: 1 addition & 1 deletion metrics/github/repos.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def get_repo_ownership(client, orgs):


def _active_repos(client, org):
return [repo for repo in query.repos(client, org) if not repo.is_archived()]
return [repo for repo in query.repos(client, org) if not repo.is_archived]


# GitHub slugs for the teams we're interested in
Expand Down
3 changes: 1 addition & 2 deletions metrics/github/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ def vulnerabilities(client, org, to_date):
for repo in repos.tech_repos(client, org):
vulns = list(map(Vulnerability.from_dict, query.vulnerabilities(client, repo)))

end = min(to_date, repo.archived_on) if repo.archived_on else to_date
for day in dates.iter_days(repo.created_on, end):
for day in dates.iter_days(repo.created_on, to_date):
closed_vulns = sum(1 for v in vulns if v.is_closed_on(day))
open_vulns = sum(1 for v in vulns if v.is_open_on(day))

Expand Down
23 changes: 2 additions & 21 deletions tests/metrics/github/test_prs.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,25 +34,6 @@ def test_makes_counts_for_every_day_between_pr_creation_and_now():
}


def test_drops_repos_after_archiving():
repo_ = repo(
"an-org",
"a-repo",
archived_on=YESTERDAY,
)
prs = {
repo_: [
pr(author="an-author", created_on=TWO_DAYS_AGO),
]
}

counts = calculate_counts(prs, true)
assert counts == {
("an-org", "a-repo", "an-author", TWO_DAYS_AGO): 1,
("an-org", "a-repo", "an-author", YESTERDAY): 1,
}


def test_counts_prs():
prs = {
repo("an-org", "a-repo"): [
Expand Down Expand Up @@ -144,12 +125,12 @@ def test_was_merged_in_on():
assert not was_merged_on(pr(merged_on=TOMORROW), TODAY)


def repo(org, name, archived_on=None):
def repo(org, name, is_archived=False):
return Repo(
org,
name,
created_on=date.min,
archived_on=archived_on,
is_archived=is_archived,
has_vulnerability_alerts_enabled=False,
)

Expand Down
8 changes: 4 additions & 4 deletions tests/metrics/github/test_repos.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def test_excludes_archived_repos(patch):
patch(
"repos",
[
repo("opensafely-core", "other-repo", archived_on=date.min),
repo("opensafely-core", "other-repo", is_archived=True),
],
)
assert len(repos.tech_repos(None, "opensafely-core")) == 0
Expand Down Expand Up @@ -121,7 +121,7 @@ def test_looks_up_ownership_across_orgs(patch):


def test_ignores_ownership_of_archived_repos(patch):
patch("repos", [repo("the_org", "the_repo", archived_on=date.min)])
patch("repos", [repo("the_org", "the_repo", is_archived=True)])
patch(
"team_repos",
{"the_org": {"team-rex": ["the_repo"], "team-rap": [], "tech-shared": []}},
Expand All @@ -139,11 +139,11 @@ def test_returns_none_for_unknown_ownership(patch):
]


def repo(org, name, archived_on=None):
def repo(org, name, is_archived=False):
return Repo(
org=org,
name=name,
created_on=date.min,
archived_on=archived_on,
is_archived=is_archived,
has_vulnerability_alerts_enabled=False,
)
28 changes: 2 additions & 26 deletions tests/metrics/github/test_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,35 +46,11 @@ def test_vulnerability_closed_on_is_closed():
assert v.is_closed_on(date(2023, 10, 29))


def test_vulnerabilities_ignores_archived_repos_after_archive_date(monkeypatch):
archive_date = date(2022, 1, 3)

def fake_repos(client, org):
return [
Repo(
"anything",
"anything",
created_on=date(2022, 1, 1),
archived_on=archive_date,
)
]

monkeypatch.setattr(security.repos, "tech_repos", fake_repos)

def fake_vulnerabilities(client, repo):
return []

monkeypatch.setattr(security.query, "vulnerabilities", fake_vulnerabilities)

result = security.vulnerabilities({}, "org", date(2022, 1, 10))
assert result[-1]["time"] == archive_date


def test_vulnerabilities(monkeypatch):
def fake_repos(client, org):
return [
Repo(org, "test", date(2023, 10, 13), None, True),
Repo(org, "test2", date(2023, 10, 13), None, True),
Repo(org, "test", date(2023, 10, 13), False, True),
Repo(org, "test2", date(2023, 10, 13), False, True),
]

monkeypatch.setattr(security.repos, "tech_repos", fake_repos)
Expand Down

0 comments on commit 4c6c4d5

Please sign in to comment.