Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2386663002:
[task scheduler] Add gen_tasks.go (Closed)
Description[task scheduler] Add gen_tasks.go
This mimics the flow of infra/bots/recipes/swarm_trigger.py
but it builds the tasks.json file instead of triggering the
actual tasks.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2386663002
Committed: https://skia.googlesource.com/skia/+/db182c770f1c1dedbc98eb00a7761706d58482e8
Patch Set 1 #
Total comments: 18
Patch Set 2 : Address comments #
Total comments: 2
Patch Set 3 : Share upload logic #
Messages
Total messages: 16 (7 generated)
Description was changed from ========== [task scheduler] Add gen_tasks.go Generates the tasks.json file. BUG=skia: ========== to ========== [task scheduler] Add gen_tasks.go Generates the tasks.json file. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2386663002 ==========
borenet@google.com changed reviewers: + benjaminwagner@google.com, borenet@google.com
Depends on https://skia-review.googlesource.com/c/2846/
This mimics the flow of infra/bots/recipes/swarm_trigger.py but it builds the tasks.json file instead of triggering the actual tasks.
Description was changed from ========== [task scheduler] Add gen_tasks.go Generates the tasks.json file. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2386663002 ========== to ========== [task scheduler] Add gen_tasks.go This mimics the flow of infra/bots/recipes/swarm_trigger.py but it builds the tasks.json file instead of triggering the actual tasks. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2386663002 ==========
Description was changed from ========== [task scheduler] Add gen_tasks.go This mimics the flow of infra/bots/recipes/swarm_trigger.py but it builds the tasks.json file instead of triggering the actual tasks. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2386663002 ========== to ========== [task scheduler] Add gen_tasks.go This mimics the flow of infra/bots/recipes/swarm_trigger.py but it builds the tasks.json file instead of triggering the actual tasks. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2386663002 ==========
https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go File infra/bots/gen_tasks.go (right): https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:74: if task_os == "Android" { Did you intend to set task_os in this if? https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:111: "pool": "Skia", nit: fmt.Sprintf("pool:%s", POOL_SKIA) (Although I would also be fine with removing POOL_SKIA.) https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:128: "GalaxyS4": "", // TODO(borenet,kjlubick) In swarm_trigger.py, this is None, which I'm guessing causes the dimension to be omitted. https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:181: return rv Sort? https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:272: // test generates a Test task. nit: say what return value is same for perf https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:347: if strings.Contains(name, "Release") { This agrees with the recipe, but just so you're aware, there's a "Perf-Ubuntu-GCC-ShuttleA-GPU-GTX550Ti-x86_64-Release-Valgrind" bot. https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:353: "--workdir", "../../..", "upload_dm_results", s/dm/nano/ https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:397: compileTaskName := deriveCompileTaskName(name, parts) This seems to assume we will have Build jobs for every Test/Perf configuration. Is that intended? https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:487: b = bytes.Replace(b, []byte("\\u003c"), []byte("<"), -1) Please add a comment.
https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go File infra/bots/gen_tasks.go (right): https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:74: if task_os == "Android" { On 2016/09/30 17:35:14, dogben wrote: > Did you intend to set task_os in this if? Yep, thanks! https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:111: "pool": "Skia", On 2016/09/30 17:35:14, dogben wrote: > nit: fmt.Sprintf("pool:%s", POOL_SKIA) > > (Although I would also be fine with removing POOL_SKIA.) Done-ish. Kept POOL_SKIA since it's used in multiple places. https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:128: "GalaxyS4": "", // TODO(borenet,kjlubick) On 2016/09/30 17:35:14, dogben wrote: > In swarm_trigger.py, this is None, which I'm guessing causes the dimension to be > omitted. Yeah, I'm not sure what the behavior is in either case. At the moment we don't have any GalaxyS4 bots, so maybe this should just be removed? https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:181: return rv On 2016/09/30 17:35:14, dogben wrote: > Sort? Done. https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:272: // test generates a Test task. On 2016/09/30 17:35:14, dogben wrote: > nit: say what return value is > > same for perf Done here and elsewhere. https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:347: if strings.Contains(name, "Release") { On 2016/09/30 17:35:14, dogben wrote: > This agrees with the recipe, but just so you're aware, there's a > "Perf-Ubuntu-GCC-ShuttleA-GPU-GTX550Ti-x86_64-Release-Valgrind" bot. Ouch. That's a bug. Fixed here, and will make a separate CL to fix the Valgrind bot. https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:353: "--workdir", "../../..", "upload_dm_results", On 2016/09/30 17:35:14, dogben wrote: > s/dm/nano/ Done. https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:397: compileTaskName := deriveCompileTaskName(name, parts) On 2016/09/30 17:35:14, dogben wrote: > This seems to assume we will have Build jobs for every Test/Perf configuration. > Is that intended? Yeah, that was my intention, although I can definitely see that changing. https://codereview.chromium.org/2386663002/diff/1/infra/bots/gen_tasks.go#new... infra/bots/gen_tasks.go:487: b = bytes.Replace(b, []byte("\\u003c"), []byte("<"), -1) On 2016/09/30 17:35:14, dogben wrote: > Please add a comment. Done.
lgtm https://codereview.chromium.org/2386663002/diff/20001/infra/bots/gen_tasks.go File infra/bots/gen_tasks.go (right): https://codereview.chromium.org/2386663002/diff/20001/infra/bots/gen_tasks.go... infra/bots/gen_tasks.go:360: skipUploadBots := []string{ Maybe share this with test().
https://codereview.chromium.org/2386663002/diff/20001/infra/bots/gen_tasks.go File infra/bots/gen_tasks.go (right): https://codereview.chromium.org/2386663002/diff/20001/infra/bots/gen_tasks.go... infra/bots/gen_tasks.go:360: skipUploadBots := []string{ On 2016/09/30 19:25:29, dogben wrote: > Maybe share this with test(). Done.
The CQ bit was checked by borenet@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from benjaminwagner@google.com Link to the patchset: https://codereview.chromium.org/2386663002/#ps40001 (title: "Share upload logic")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [task scheduler] Add gen_tasks.go This mimics the flow of infra/bots/recipes/swarm_trigger.py but it builds the tasks.json file instead of triggering the actual tasks. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2386663002 ========== to ========== [task scheduler] Add gen_tasks.go This mimics the flow of infra/bots/recipes/swarm_trigger.py but it builds the tasks.json file instead of triggering the actual tasks. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2386663002 Committed: https://skia.googlesource.com/skia/+/db182c770f1c1dedbc98eb00a7761706d58482e8 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://skia.googlesource.com/skia/+/db182c770f1c1dedbc98eb00a7761706d58482e8 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
