"GitHub Comments Missing"

[Allen Wittenauer <aw...@effectivemachines.com.INVALID>]

	Since someone asked me privately, thought I'd make sure that people know that if you are using the main branch that YETUS-994 did away with the GitHub comment report and replaced it with a GitHub Status.  If you don't see GitHub Statuses from test-patch, most likely explanation is that the token you are using doesn't have repo:status write access.  Once you hand test-patch a token with that capability, you should start seeing Yetus stuff in branch and PRs like you do for other GitHub integrated bits.

Re: "GitHub Comments Missing"

[Allen Wittenauer <aw...@apache.org>]

> On Sep 29, 2020, at 9:17 AM, Allen Wittenauer <aw...@effectivemachines.com.INVALID> wrote:
> 
>>> Maybe test-patch should complain if it has a token that lacks this permission?
>> 
>> AFAIK, GitHub returns 404 if the token lacks the permission, so
>> test-patch cannot complain about the permission of the token.
> 
> 	You can also get a 404 for other reasons though. :(

	After reworking my local jenkins instance to be configured correctly for statuses on forked PRs (moving from personal access token to GitHub app token, etc), it wasn't a 404. It was a 422 which is a completely different problem.  After applying YETUS-1022, statuses are actually working better in Yetus than they are from Jenkins. haha. But given some of the weird rules around the time tokens live, that still may not work for Hadoop, HBase, etc. Not much can be done about that other than making build, test, etc run faster.

Re: "GitHub Comments Missing"

[Allen Wittenauer <aw...@effectivemachines.com.INVALID>]

> On Sep 29, 2020, at 3:56 AM, Akira Ajisaka <aa...@apache.org> wrote:
> 
> I'm testing YETUS-994 with a token with repo:status write access,
> however, Yetus couldn't update the commit status as expected.
> https://github.com/apache/hadoop/pull/2348

Try a patch with failures.  A status is only put if there are warnings or failures.  If the patch is good, then there is an assumption that the CI system will also be putting up a status (which, in this case, it did).  We can always change the code to always put a good status, but keep in mind that there are limits as to how many statuses (statusii?) one is allowed to have...

> 
>> Also discovered this morning that Jenkins doesn't always set the GIT_COMMIT env variable which will mess things up too.
> 
> Is this the root cause of the failure?

	Nope. Doesn't look like there was a failure for it to report.  There is actually a message if the SHA is missing.  YETUS-1006 now has a PR which should fix this problem btw.

> 
>> Maybe test-patch should complain if it has a token that lacks this permission?
> 
> AFAIK, GitHub returns 404 if the token lacks the permission, so
> test-patch cannot complain about the permission of the token.

	You can also get a 404 for other reasons though. :(

Re: "GitHub Comments Missing"

[Akira Ajisaka <aa...@apache.org>]

I'm testing YETUS-994 with a token with repo:status write access,
however, Yetus couldn't update the commit status as expected.
https://github.com/apache/hadoop/pull/2348

> Also discovered this morning that Jenkins doesn't always set the GIT_COMMIT env variable which will mess things up too.

Is this the root cause of the failure?

> Maybe test-patch should complain if it has a token that lacks this permission?

AFAIK, GitHub returns 404 if the token lacks the permission, so
test-patch cannot complain about the permission of the token.

-Akira

On Tue, Sep 29, 2020 at 4:20 AM Allen Wittenauer
<aw...@effectivemachines.com.invalid> wrote:
>
>
>
> > On Sep 28, 2020, at 12:08 PM, Nick Dimiduk <nd...@apache.org> wrote:
> >
> > Maybe test-patch should complain if it has a token that lacks this
> > permission?
>
>         I actually had some code to do that but yanked it.  On a few trial runs with GitHub Actions when using the built-in token, /authorize wasn't actually setting the X-OAuth-Header that tells what scopes that token has. :( [This feels like a bug in GitHub but I want to verify it with a few different scenarios--esp Jenkins using a GitHub App--before I pester GitHub about it.]
>
>         I do know that (usually) Jenkins itself complains about it because it will also try to update repo status.
>
>         Also discovered this morning that Jenkins doesn't always set the GIT_COMMIT env variable which will mess things up too.  That's easier to fix/workaround though.
>
>

Re: "GitHub Comments Missing"

[Allen Wittenauer <aw...@effectivemachines.com.INVALID>]

> On Sep 28, 2020, at 12:08 PM, Nick Dimiduk <nd...@apache.org> wrote:
> 
> Maybe test-patch should complain if it has a token that lacks this
> permission?

	I actually had some code to do that but yanked it.  On a few trial runs with GitHub Actions when using the built-in token, /authorize wasn't actually setting the X-OAuth-Header that tells what scopes that token has. :( [This feels like a bug in GitHub but I want to verify it with a few different scenarios--esp Jenkins using a GitHub App--before I pester GitHub about it.]  

	I do know that (usually) Jenkins itself complains about it because it will also try to update repo status.

	Also discovered this morning that Jenkins doesn't always set the GIT_COMMIT env variable which will mess things up too.  That's easier to fix/workaround though.

Re: "GitHub Comments Missing"

[Nick Dimiduk <nd...@apache.org>]

Maybe test-patch should complain if it has a token that lacks this
permission?

On Sun, Sep 27, 2020 at 12:47 PM Allen Wittenauer
<aw...@effectivemachines.com.invalid> wrote:

>
>         Since someone asked me privately, thought I'd make sure that
> people know that if you are using the main branch that YETUS-994 did away
> with the GitHub comment report and replaced it with a GitHub Status.  If
> you don't see GitHub Statuses from test-patch, most likely explanation is
> that the token you are using doesn't have repo:status write access.  Once
> you hand test-patch a token with that capability, you should start seeing
> Yetus stuff in branch and PRs like you do for other GitHub integrated bits.