Return all open PRs instead of filtering by date#4
Merged
Conversation
This resource can inadvertently miss Pull Requests due to out-of-order commits across PRs. If PR#2 is opened after PR#1, but the head commit of PR#2 is older than the head commit of PR#1, the resource will not include PR#2 in the list of new versions provided to Concourse. Rather than attempt to find a different way of tracking which PRs are "new" given an input version, we can remove the date-based filtering and return all open PRs. Concourse can deduplicate versions based on metadata, which means that we will only trigger new jobs for versions that Concourse hasn't seen before. This makes it easier for teams to use this resource to track PRs in Concourse, since they no longer have to ensure that a PR has a later head commit than all currently-opened PRs in order to notify Concourse that their PR exists. (cherry picked from commit 50ef79a)
Author
|
I'll try testing / deploying this tomorrow when concourse pipelines are unpaused. |
53fdfe2 to
e7c80e8
Compare
Author
|
To confirm this bug is fixed, I:
This matches the reproduction described in telia-oss#26 (comment). So, this resolves the issue we used On top of that, it reduces the API usage of the github token by about half, no longer hitting the limit. I tried turning this feature on & off, and you can see the difference here (datadog link): I'm merging this PR and making it the |
This was referenced Dec 10, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.




From the original commit:
This problem is not fixed upstream, but it is discussed in telia-oss#205. There are a few solutions provided:
digitalocean/github-pr-resourceRight now we're using solution 1, using
opendoor/digitalocean-github-pr-resourcein only the code pipeline. In all other pipelines we useopendoor/telia-oss-github-pr-resource, which is based on the master branch in this repo and contains caching improvements to reduce GitHub API token usage.Instead of using two different version, I've applied ctreatma@50ef79a to our branch, fixed the merge conflicts and made tests pass. Once this is merged in, all pipelines can use a single version of this resource which has all the required features:
This fixes the issue by not filtering on date at all, with deduplication happening at the DB layer in concourse. In theory this will increase the DB resource usage, but that's sitting at 5% today and has never been a bottleneck.
Hopefully the api caching part will reduce the GitHub API token usage of the
codepipeline, which is almost at the limit of a single user already.Test Plan
go test -race -v ./...test manually
bm-fix-telia-oss-26latestlatestTo deploy:
bm-fix-telia-oss-26tolatest