Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add branch (headRefName) source filter #254

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chrisfarms
Copy link

@chrisfarms chrisfarms commented Mar 30, 2021

What

Allows setting a "branch" in the source configuration, that filters the PR versions down to only those with a matching headRefName.

Why

When using instanced pipelines (one-pipeline-per-pr) it is desirable to monitor changes only for an individual PR and not all PRs. Pinning the resource version is not suitable for this use-case as the version contains the "commit" (we still want the resource to trigger on commit changes, we just only want to track those for a single PR).

Ideally we could filter by PR-number or some other unique ID, however the GraphQL API does not expose this, and we want to avoid filtering client-side.

Note

There should be a one-to-one mapping of headRefName to PR in most real-world usage, and so for this "one-pipeline-per-pr" use case, it is probably "good enough". However there is nothing stopping branch names getting reused over time, so you should not consider setting "branch" the same as "pinning" a resource version, it is merely a filter like state.

Allows setting a "branch" in the source configuration, that filters the
PR versions down to only those with a matching headRefName.

When using instanced pipelines (one-pipeline-per-pr) it is desirable to
monitor changes only for an individual PR and not all PRs.

Ideally we could filter by PR-number or some other unique ID, however
the GraphQL API does not expose this, and we want to avoid filtering
client-side.

Note: There _should_ be a one-to-one mapping of headRefName to PR in
most real-world usage, and so for this "one-pipeline-per-pr" use case,
it is probably "good enough". However there is nothing stopping branch
names getting reused over time, so you should not consider setting
"branch" the same as "pinning" a resource version, it is merely a
filter.
@chrisfarms chrisfarms requested a review from a team as a code owner March 30, 2021 16:59
@vixus0
Copy link
Contributor

vixus0 commented May 12, 2021

@chrisfarms Does Concourse's partial version pinning help in this case (concourse/concourse#4380)? I think that would allow you to pin version.pr. I may be misunderstanding how the partial version pinning works though.

@chrisfarms
Copy link
Author

Does Concourse's partial version pinning help in this case

Interesting @vixus0 .... I think could ... but the pr-resource does not seem to emit ALL versions during check, and so you get a state where the only version in the list returned from check would be for some other random PR won't match the version at all, and it becomes a game of probability wether it works or not.

By explicitly scoping the resource to a single PR (well branch) you get to treat the PR-resource more like you treat the git-resource, following commits on a single branch and not caring about other unrelated PRs as they change state.

@vixus0
Copy link
Contributor

vixus0 commented Jun 1, 2021

Yeah also I realise now the partial pinning results in weirdness with get steps passed param. In any case I'd like to see this PR merged 💯

@Mike-Dax
Copy link

Mike-Dax commented Jul 6, 2021

We tried the partial pinning approach and found new commits on the PRs don't trigger new builds, so I'd be interested in having this merged and released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants