Close #26: Filter PRs by updated date instead of commit date #261
Conversation
|
Hi, Thanks for the contribution, when I get the time, I'll see if these changes resolve our issues. I had the same implementation in private branch of this, but I made the mistake of digging myself a hole trying to make some of the queries optional for other features. But going back to the basics is a great idea. Do you have an image available of this ready to be tested? |
|
@rickardl Can this be addressed sooner, I have bunch of PRs opened some time back and label added today not triggering build. |
|
I'm somewhat waiting for a confirmation that this resolves the issue, I haven't seen a result from the test suite being run here. |
This supersedes #205. 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. In #205, I removed the date filter entirely. This ensures that the PR resource will find all PRs that match the explicitly-configured filters. While Concourse can detect and ignore duplicate versions, it has to run a database query for every version returned by a `check`, so removing the date filter entirely would increase load on a Concourse database. (That said, I'm not sure whether this increased load is a particular concern, and other resources don't seem to make much effort to avoid returning duplicate versions from a `check`.) To avoid that extra load on a Concourse database, this change instead replaces the filter by commit date in `check.go` with a filter by updated date in the GraphQL query to list pull requests. This should reduce the number of duplicate versions returned by a `check` while still allowing the PR resource to detect PRs with out-of-order head commits.
ctreatma
force-pushed
the
feature/date_filter_graphql
branch
from
c9ee589
to
4f3ff8d
Compare
|
GitHub says it's not going to run the tests for this PR until a maintainer approves them. |
|
Hello, I made my own docker image based on this PR, and it solved our issue, the PR that wasn't seen by the CI, even with fake git commit is now seen correctly and has been processed by our CI 🎉 |
|
Hello, any news on this one? |
|
I'm tired of waiting, test is still running, people are yelling. Started to write my own PR resource. 😂 |
|
Is there any update on this? We are facing the same issue. |
|
Is there any way a maintainer can allow the test jobs to run so we can confirm if this PR is mergeable? I ran the tests locally and they passed, but they won't run in GitHub unless a maintainer approves them. |
|
Any updates on this? I'm pretty sure this passes all of the tests, but I the PR checks won't run unless a maintainer allows them to run. We continue to run into this issue on a regular basis and would prefer to have an official release with the fix rather than running a fork. |
|
@ctreatma |
This supersedes #205.
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.
In #205, I removed the date filter entirely. This ensures that the PR
resource will find all PRs that match the explicitly-configured filters.
While Concourse can detect and ignore duplicate versions, it has to run
a database query for every version returned by a
check, so removingthe date filter entirely would increase load on a Concourse database.
(That said, I'm not sure whether this increased load is a particular
concern, and other resources don't seem to make much effort to avoid
returning duplicate versions from a
check.)To avoid that extra load on a Concourse database, this change instead
replaces the filter by commit date in
check.gowith a filter by updateddate in the GraphQL query to list pull requests. This should reduce the
number of duplicate versions returned by a
checkwhile still allowingthe PR resource to detect PRs with out-of-order head commits.