New issue
Advanced search Search tips

Issue 750694 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 747960



Sign in to add a comment

Error on LUCI v8_presubmit

Project Member Reported by machenb...@chromium.org, Jul 31 2017

Issue description

See:
https://chromium-review.googlesource.com/c/590449/1

Experimental luci builder fails while non-luci succeeds. Repros locally with:

./scripts/slave/recipes.py run --properties-file - run_presubmit <<EOF
{"recipe": "run_presubmit", "patch_ref": "refs/changes/49/590449/1", "patch_project": "v8/v8", "$recipe_engine/path": {"cache_dir": "/b/swarming/w/ir/cache", "temp_dir": "/b/swarming/w/ir/tmp/rt"}, "runhooks": true, "patch_gerrit_url": "https://chromium-review.googlesource.com", "dry_run": "true", "patch_repository_url": "https://chromium.googlesource.com/v8/v8", "swarming_run_id": "37a12e376b59e811", "attempt_start_ts": 1501230982000000, "blamelist": ["jgruber@chromium.org"], "master": "master.tryserver.v8", "repo_name": "v8", "category": "cq_experimental", "patch_set": 1, "repository": "https://chromium.googlesource.com/v8/v8", "buildername": "v8_presubmit", "mastername": "tryserver.v8", "patch_storage": "gerrit", "buildbucket": {"build": {"created_ts": 1501231361670390, "bucket": "luci.v8.try", "id": "8972838539663754592", "created_by": "user:luci-migration@appspot.gserviceaccount.com", "tags": ["buildset:patch/gerrit/chromium-review.googlesource.com/590449/1", "luci_migration_attempt:2", "luci_migration_buildbot_build_id:8972838919772159232", "user_agent:luci-migration"]}}, "reason": "CQ", "patch_issue": 590449, "$recipe_engine/step": {"prefix_path": ["/b/swarming/w/ir/cipd_bin_packages", "/b/swarming/w/ir/cipd_bin_packages/bin"]}, "path_config": "generic", "revision": "f4b2b9bef77c35491adc6c56be157c09294f79ba", "bot_id": "swarm915-c4"}
EOF
 
Cc: d...@chromium.org

Comment 2 by d...@chromium.org, Jul 31 2017

Cc: aga...@chromium.org
Components: -Infra>Platform Infra>Codereview>Gerrit
This actually looks like a bug in "presubmit_support.py". It's expecting the "Code-Review" label to be present in the JSON response, but it's not. Looking at the CL in question, the label is not present, likely because the change is abandoned.

+agable@, who is working on Gerrit integration into our tooling. I suspect this is just adding code acknowledging that "Code-Review" is not always present.

Comment 3 by d...@chromium.org, Jul 31 2017

Oh actual error log: https://luci-logdog.appspot.com/v/?s=v8%2Fbuildbucket%2Fcr-buildbucket.appspot.com%2F8972838539663754592%2F%2B%2Fsteps%2Fpresubmit%2F0%2Fstdout

DEBUG:root:Running CheckChangeOnCommit in /b/swarming/w/ir/kitchen-workdir/v8/PRESUBMIT.py
Traceback (most recent call last):
  File "/b/swarming/w/ir/kitchen-checkout/scripts/slave/.recipe_deps/depot_tools/presubmit_support.py", line 1621, in <module>
    sys.exit(main())
  File "/b/swarming/w/ir/kitchen-checkout/scripts/slave/.recipe_deps/depot_tools/presubmit_support.py", line 1605, in main
    options.dry_run)
  File "/b/swarming/w/ir/kitchen-checkout/scripts/slave/.recipe_deps/depot_tools/presubmit_support.py", line 1348, in DoPresubmitChecks
    results += executer.ExecPresubmitScript(presubmit_script, filename)
  File "/b/swarming/w/ir/kitchen-checkout/scripts/slave/.recipe_deps/depot_tools/presubmit_support.py", line 1258, in ExecPresubmitScript
    result = eval(function_name + '(*__args)', context)
  File "<string>", line 1, in <module>
  File "<string>", line 348, in CheckChangeOnCommit
  File "<string>", line 269, in _CommonChecks
  File "/b/swarming/w/ir/kitchen-checkout/scripts/slave/.recipe_deps/depot_tools/presubmit_canned_checks.py", line 864, in CheckOwners
    approval_needed=input_api.is_committing)
  File "/b/swarming/w/ir/kitchen-checkout/scripts/slave/.recipe_deps/depot_tools/presubmit_canned_checks.py", line 911, in GetCodereviewOwnerAndReviewers
    return func(input_api, email_regexp, approval_needed)
  File "/b/swarming/w/ir/kitchen-checkout/scripts/slave/.recipe_deps/depot_tools/presubmit_canned_checks.py", line 972, in _GerritOwnerAndReviewers
    r for r in input_api.gerrit.GetChangeReviewers(issue, approval_needed)
  File "/b/swarming/w/ir/kitchen-checkout/scripts/slave/.recipe_deps/depot_tools/presubmit_support.py", line 250, in GetChangeReviewers
    cr = self.GetChangeInfo(issue)['labels']['Code-Review']
KeyError: 'Code-Review'
Ah, so it was a race between the non-luci bot and the experimental luci version and the CL got abandoned right in between?

Comment 5 by d...@chromium.org, Jul 31 2017

I think so, yeah.

Comment 6 by aga...@chromium.org, Jul 31 2017

Owner: aga...@chromium.org
Status: Started (was: Untriaged)
Yep, exactly. I can work on a change to PRESUBMIT to not fail in quite this way: https://chromium-review.googlesource.com/594839

But also it shouldn't be missing just because the change is abandoned. That has to be a bug. If it's a feature, it's a bad one: b/64207819
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/8b478f04067599177cc5b064c81f0a1e915095f0

commit 8b478f04067599177cc5b064c81f0a1e915095f0
Author: Aaron Gable <agable@chromium.org>
Date: Mon Jul 31 23:25:14 2017

PRESUBMIT: Be resilient to changes with no Code-Review label

Bug:  750694 
Change-Id: I69778c3ea0789795ad7e50d11bf677df8f7bc262
Reviewed-on: https://chromium-review.googlesource.com/594839
Reviewed-by: Daniel Jacques <dnj@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>

[modify] https://crrev.com/8b478f04067599177cc5b064c81f0a1e915095f0/presubmit_support.py
[modify] https://crrev.com/8b478f04067599177cc5b064c81f0a1e915095f0/tests/presubmit_unittest.py

Comment 8 by aga...@chromium.org, Sep 11 2017

Status: Fixed (was: Started)

Sign in to add a comment