x/build/maintner: Maintner does not detect transferred issues #37370
Labels
Builders
x/build issues (builders, bots, dashboards)
NeedsInvestigation
Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Comments
toothrot
added
the
NeedsInvestigation
|
I implemented my suggestion, and discovered that GitHub doesn't actually add Instead, I'm reverting back to relying on https://golang.org/cl/161521/ as a partial fix. |
|
@dmitshur has suggested that we could check the endpoint https://api.github.com/repos/golang/vscode-go/issues/144/events (for an issue like https://github.com/golang/vscode-go/issues/144). Then, we could just skip adding any issue that no longer exists. |
|
Change https://go.dev/cl/397234 mentions this issue: |
gopherbot
pushed a commit
to golang/build
that referenced
this issue
Apr 1, 2022
When synchronizing comments of an issue, getting a 404 (or 410) from GitHub REST API's list comments endpoint should happen if and only if the issue no longer exists. Use that as a safe opportunity to append a mutation to the log that updates the issue's NotExist field to true. Maintner doesn't yet have complete support for transferred issues, a feature that GitHub added after its initial design. It's been difficult to add it retroactively as described in go.dev/issue/37603. This change aims to make a small incremental change that we can feel confident about applying and watching closely, and doesn't try to implement a complete fix. It should help get maintner unstuck when trying to sync comments in an issue tracker that has a transferred issue, and incurs a minimal risk of writing bad mutations. This CL is based on past work in CL 161521 and CL 176638, but is smaller in scope intentionally. For golang/go#37370. For golang/go#40640. Updates golang/go#45461. Fixes golang/go#52017. Change-Id: I3702d3c7b2e412cf75809fdb722c0ee693c8600e Reviewed-on: https://go-review.googlesource.com/c/build/+/397234 Trust: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Related to #30184.
Maintner's sync loop uses a ListIssues call sorted by update time to determine which issues it needs to re-fetch issue data and event data for. Deleted issues and transferred issues don't show up in the ListIssues response, Maintner is never aware that they're gone, and ultimately those issues are frozen in the corpus in the state they had before they were deleted/transferred. They are never marked NotExist (tombstoned). https://go-review.googlesource.com/c/build/+/161521/ is an insufficient fix for this reason.
https://go-review.googlesource.com/c/build/+/205598 proposes exposing a mutation method so an external process can inform Maintner of the deleted/transferred issues, but that requires an external process (also, see other comments on that CL).
I think a better solution would be for Maintner, during its issue sync, to pull repository events up to the GitHub poller's lastUpdated timestamp, looking specifically for issue transfer and issue delete events, and tombstoning (mark as
NotExist) the appropriate issues before moving onto the regular issue sync.The difficulty will be for repos that are already being synced, this doesn't provide a way to go back in time and handle previous deletes/transfers, which, the external process would be capable of doing.
/cc @bradfitz @dmitshur @orthros
The text was updated successfully, but these errors were encountered: