New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 825551 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: 2019-01-24
OS: ----
Pri: 1
Type: Feature



Sign in to add a comment

Extend Gitiles trigger to support subpaths

Project Member Reported by phosek@chromium.org, Mar 25 2018

Issue description

We would like to extend the Gitiles trigger to support paths, to only trigger a job when the specified path within the repository changes. This is useful for very large repositories containing number of subprojects where individual subprojects have their own builders. Example use case of this feature is:


trigger {
  id: "gn-build"
  acl_sets: "internal"
  gitiles: {
    repo: "https://chromium.googlesource.com/chromium/src"
    refs: "refs/heads/master"
    path: "tools/gn"
  }
  triggers: "gn-builder-linux"
  triggers: "gn-builder-win"
  triggers: "gn-builder-mac"
}

The Gitiles trigger uses Gitiles /+refs/$ref endpoint to find out if any of the refs have changed. When the path is specified, we'd use /+log/$old..$new/$path to find out if the path has changed ($old is stored in the datastore, $new is the result of /+refs/$ref).
 

Comment 1 by no...@chromium.org, Mar 28 2018

Labels: -Restrict-View-Google Type-Feature
Sounds reasonable. Andrii, Vadim?
Labels: Pri-2
Status: Available (was: Untriaged)
SGTM.
We also have a use case for an "any path but" blacklist, maybe call it "ignore_path" if the whitelist is called "path", or make them "include"/"exclude".

That is, if the only paths touched in the $old..$new range match an ignore_path pattern, then don't trigger.  (This is for a repo that has useless mechanical commits more often than real changes.)
Cc: bpastene@chromium.org
+1

I'm also running into an instance where a filepath filter in the trigger would be useful. (Right now my recipe is a cron that just no-ops if the filepath hasn't changed since the last time it ran. Would be nice to instead trigger on changes to that filepath.) Not actively blocking anything tho.
Labels: ParallelCQ
This is now critical to Chrome OS’s ParallelCQ effort because they need to watch a single file
Labels: -Pri-2 Pri-1
Owner: tandrii@chromium.org
Status: Assigned (was: Available)
OK, assigning my way.
I've briefly looked at what's easiest way to implement it, and there are two approaches:

1. What Petr wrote in #0, namely let gitiles do filter by using 
  /+log/$old..$new/$path

   - requires checking corner cases of gitiles to ensure code around corner cases can stay the same (e.g., what happens on force push? merge commit (see also internal http://b/122868467). Still, with luck doable anyway, plug gitiles bugs (if any) should be fixed anyway
  ++ very efficient on scheduler side
  -- not flexible (but can't be abused!) - e.g., no blacklists.


2. Ask gitiles to return tree diff structure alongside each commit.

  ++ easy to implement
  ++ supports arbitrary filepath filtering
   - more load on gitiles, more network IO.
   - tree diff structure will increase memory usage, however given current scheduler limits of how many commits to look back, this is minor.


== I'm leaning towards 2.

Vadim et al, WDYT?
also, nodir@ re #c5: where/whom do I learn from exact requirement of CrOS? Is it just a "path: not-a-regexp" sufficient?
Just a path would be sufficient. the context is go/converge-browser-os-infra where a poller must watch changes of one manifest file. Note that they keep multiple manifests in one repo
Considering 2 is "easy to implement" and potentially more flexible, we can start with it and then reconsider if we see any performance issues.

Comment 12 by tandrii@chromium.org, Jan 18 (4 days ago)

NextAction: 2019-01-24
Status: Started (was: Assigned)
I've thought through implementation and there are some limitations, so I documented them in our config proto and sent for review. PTAL && comment if you care about this feature:

https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/1423099

Sign in to add a comment