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

Issue 698625 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

depot_tools.py: gerrit_util.py uses outdated feature 'sortkey'

Project Member Reported by jmad...@chromium.org, Mar 6 2017

Issue description

Andrii maybe you could help with this or find an owner. I tried to fix it myself but wasn't able to make it work.

When QueryChanges finds there are more CLs than fits the limit, it tries to use an outdated Gerrit feature 'sortkey'. This has been replaced by a 'skip' parameter. 

To repro, you'll need to call QueryChanges with a filter that returns a lot of CLs, such as every change to ANGLE ever (which is what I was trying). I can upload my (bad) repro case if needed.

See this stackoverflow page for the error and solution:
http://stackoverflow.com/questions/24606079/gerrit-rest-api-cannot-use-sortkey-to-resume-a-query
 
Components: -Infra Infra>Platform>CQdaemon Infra>Codereview>Gerrit Infra>SDK
Labels: -Pri-2 Pri-1
Status: Started (was: Assigned)
THanks for filing! I thought I can just copy-paste code from here:

https://cs.chromium.org/chromium/infra/infra/libs/gerrit_api/gerrit_api.py?l=339

(which is what Gerrit CQ is using), and then realized there is no paging at all there even though I was 100% it is! This is horrifying, so marking as P1 for CQ to fix, and I'll patch depot_tools in the process.
No problem, thank you as well for the fast response.
Cc: andyb...@chromium.org
Labels: Milestone-Dogfood Proj-Gerrit-Migration
fyi Andy: we can't dogfood chromium until this is fixed, else CQ click latency will be HUGE.
Cc: aga...@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 6 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/7514e3f60d6e18d1160c97d97b0038b134ad674f

commit 7514e3f60d6e18d1160c97d97b0038b134ad674f
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Mon Mar 06 17:54:06 2017

Update: I sounded alarm too loudly. Gerrit will happily return up to 10k in one request, so CQ is fine (although a few Cls will land to detect if CQ is reaching boundary that require re-engineering).

As for depot_tools, if there are <10k CLs, it should be fine to just use higher limit.
Preliminary fix for depot_tools and my_activity: https://chromium-review.googlesource.com/c/451383 

it's not complete yet, but jmadill@ -> maybe it's useful for your case.
Thanks Andrii, I actually was under 10k CLs since 2014 so I was able to get it working by editing the hard-coded limit up to 10,000, thanks!
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 8 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/bf28c4ad7d4d414427be95eaa5311a5b1deaf73f

commit bf28c4ad7d4d414427be95eaa5311a5b1deaf73f
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Wed Mar 08 14:55:43 2017

Project Member

Comment 11 by bugdroid1@chromium.org, Mar 8 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/7ab22b5530bd885f6359c2166e81ca031e0fa638

commit 7ab22b5530bd885f6359c2166e81ca031e0fa638
Author: Andrii Shyshkalov <tandrii@google.com>
Date: Wed Mar 08 15:08:27 2017

Project Member

Comment 12 by bugdroid1@chromium.org, Mar 8 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/b6e57f8fac7a808e24ae5df283ac331deca0d918

commit b6e57f8fac7a808e24ae5df283ac331deca0d918
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Wed Mar 08 15:20:40 2017

Labels: -Pri-1 -Milestone-Dogfood Milestone-Launch Pri-2
What's left? Landing this CL: https://chromium-review.googlesource.com/c/451383 
Project Member

Comment 14 by bugdroid1@chromium.org, Mar 13 2017

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

commit 892e9c267448810e65ab6ecd03e99c57293b16e3
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Mon Mar 13 14:14:24 2017

gerrit_util and my_activity: fix fetching >400 changes from Gerrit.

BUG= 698625 

Change-Id: I892f09e8b7ff752b6a45c556a594f486000530e8
Reviewed-on: https://chromium-review.googlesource.com/451383
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/892e9c267448810e65ab6ecd03e99c57293b16e3/gerrit_util.py

Status: Fixed (was: Started)
Thanks again Andrii!

Sign in to add a comment