Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

Incorrect PR Size label #1892

Closed
saad-ali opened this issue Oct 19, 2016 · 8 comments · Fixed by #1903
Closed

Incorrect PR Size label #1892

saad-ali opened this issue Oct 19, 2016 · 8 comments · Fixed by #1903
Labels
area/mungegithub kind/bug Categorizes issue or PR as related to a bug.

Comments

@saad-ali
Copy link
Contributor

In PR kubernetes/kubernetes#33944 the bot correctly applied the size/XXL label to the PR that touches 72 files and adds 6936 lines.

It later removed that label and replaced it with size/XS <-- Buggy behavior

@saad-ali saad-ali added the kind/bug Categorizes issue or PR as related to a bug. label Oct 19, 2016
@saad-ali
Copy link
Contributor Author

Another example where the bot seems to be fighting itself: PR kubernetes/kubernetes#28599

@foxish
Copy link
Contributor

foxish commented Oct 19, 2016

@eparis any idea why this might happen? A github api glitch? I would guess so, because it is adding XS which means it thinks the number of lines changed is < 10.

@eparis
Copy link
Contributor

eparis commented Oct 19, 2016

The sizer is supposed to ignore all changes in vendor so it should be possible for a HUGE PR that only changes vendor to be XS.

But it is obviously broken... Somehow the amount of changes in vendor are being mis-counted...

@eparis
Copy link
Contributor

eparis commented Oct 19, 2016

Seems definitely like a github API bug...

I'm looking specifically at kubernetes/kubernetes#28599.

If I look at the api
https://api.github.com/repos/kubernetes/kubernetes/pulls/28599

I'll see that is says:

  "additions": 18570,
  "deletions": 239,

But if I look at it via git:

git diff --stat $(git merge-base upstream/pr/28599 upstream/master)..upstream/pr/28599

[snip]
 167 files changed, 62513 insertions(+), 41876 deletions(-)

So the github api says there are only 18k insertions but there are really 65k insetions! When we then remove the additions/deletions from ignores paths (in this case that includes at least vendor and more importantly changes to pkg/api/types.generated.go and pkg/api/v1/types.generated.go which BOTH have > 20k additions and deletions) we end up with a negative size.

Additions: -39003 Subtractions: -40594

No idea why the github api just lies about the # of Additions and Deletions.

@eparis
Copy link
Contributor

eparis commented Oct 19, 2016

e-mail sent to github support

@foxish
Copy link
Contributor

foxish commented Oct 19, 2016

@eparis Thanks for figuring it out. I wonder it is a recent change on their end that caused this.

@eparis
Copy link
Contributor

eparis commented Oct 20, 2016

Hi Eric,

If you check https://github.com/kubernetes/kubernetes/pull/28599/files you'll see that the UI shows the same thing (so it's not a problem in the API) and offers an explanation about why the numbers are like that:

"Sorry, we could not display the entire diff because it was too big."

And for that file pkg/api/types.generated.go specifically you'll see:

"20,953 additions, 20,281 deletions not shown because the diff is too large. Please use a local Git client to view these changes."

You're hitting some documented limits in displaying and counting diffs:

https://help.github.com/articles/what-are-the-limits-for-viewing-content-and-diffs-in-my-repository

It's something we'd like to improve in the future, but I can't promise an ETA.

Cheers,
Ivan

@eparis
Copy link
Contributor

eparis commented Oct 20, 2016

Maybe we should calculate the size summing the file.Additions instead of using the pr.Additions and then subtracting out the file.Additions we wish to ignore?

@eparis eparis mentioned this issue Oct 20, 2016
k8s-github-robot pushed a commit that referenced this issue Oct 20, 2016
Automatic merge from submit-queue

Fix size check

fixes #1892 

The github api lies about the size of diffs if they are large. If we
then subtract out the size of generated files that it is not lieing
about we end up with negative numbers. So huge PRs can get XS size.

Instead of believing the API and deleting those things we don't care
about just sum the things we do care about and use that.
foxish pushed a commit to foxish/contrib that referenced this issue Jan 20, 2017
Automatic merge from submit-queue

Fix size check

fixes kubernetes-retired#1892 

The github api lies about the size of diffs if they are large. If we
then subtract out the size of generated files that it is not lieing
about we end up with negative numbers. So huge PRs can get XS size.

Instead of believing the API and deleting those things we don't care
about just sum the things we do care about and use that.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/mungegithub kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants