dvecy opened a new pull request, #9244:
URL: https://github.com/apache/devlake/pull/9244
### Summary
`Collect Pull Requests` has a second phase that re-fetches pull requests one
by one to refresh
the ones that are not finished yet. It picked them with:
```sql
state != 'closed' OR merged = false
```
A pull request that was closed without being merged never becomes `merged`,
so it keeps matching
for ever: one `GET /pulls/{number}` for it on every single run, for as long
as the repo exists.
#9240 reports ~15k of those on one instance, the same payload stored again
and again, and a run
finally dying on a `422` from GitHub.
### Why that condition was there
`/pulls` (the list endpoint) does not return `merged`, `commits`,
`additions`, `deletions`,
`review_comments` or `merged_by`. So after a full sync every closed pull
request is stored
without them, and `OR merged = false` (44f2db27d) was added to fetch each
one once more and fill
them in. That repair is still needed — it just never stopped.
### Fix
Select on `commits = 0` instead of `merged = false`. Every pull request has
at least one commit,
so `commits = 0` means "details were never collected", and it stops being
true as soon as they
are. Exactly one case changes: a closed, never merged pull request whose
details are already in
the database is no longer re-fetched. Open pull requests, merged ones, and
the repair pass after
a full sync behave as before. No schema change, no migration, no config
change.
The query moved into `UnfinishedPullRequestsClauses` so the test runs the
collector's own query
instead of a copy of it.
### One thing to be aware of
A closed pull request that is reopened later is no longer picked up by an
incremental run. That
is already the case for merged ones, `github_graphql` does the same (it only
refreshes
`state = 'OPEN'`), and a full sync reconciles it. Catching reopens
incrementally would need the
list phase to page by `sort=updated` — happy to file a follow-up issue if
you want that tracked.
### Tests
New `TestGithubUnfinishedPullRequests` seeds one pull request per case and
asserts the exact set
the query returns. On `main` it fails with `[1 2 4 5 6]` instead of `[1 2 4
5]`; with the fix it
passes.
Locally against PostgreSQL 16: `go test ./plugins/github/...` all packages
ok, `go vet` clean,
`golangci-lint run ./plugins/github/...` 0 issues, `gofmt -l
plugins/github/` empty. The
condition is plain portable SQL, so MySQL and PostgreSQL behave the same.
### Does this close any open issues?
Closes #9240
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
