Re: [macports-infrastructure] 02/02: buildbot: Update status on GitHub after each build

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [macports-infrastructure] 02/02: buildbot: Update status on GitHub after each build

Ryan Schmidt-24

On Mar 10, 2018, at 11:42, Rainer Müller wrote:

> Rainer Müller (raimue) pushed a commit to branch master
> in repository macports-infrastructure.
>
>
> https://github.com/macports/macports-infrastructure/commit/d9aeeb32b22a06eb39f1a0317869e9c12e3ed021
>
> commit d9aeeb32b22a06eb39f1a0317869e9c12e3ed021
>
> Author: Rainer Müller
> AuthorDate: Sat Mar 10 18:05:21 2018 +0100
>
>
>     buildbot: Update status on GitHub after each build
>
>    
>
>     This adds a status icon next to the commit in the commit log on the
>
>     GitHub web interface. The GitHub API key has to have at least
>
>     repo:status scope.
>
>    
>
>     See: https://developer.github.com/v3/repos/statuses/

This is deployed, but each completed build's status overwrites the status of the previous build. So which build you'll see as the commit status is random depending on which builder takes the longest. Which isn't really random; it's gonna be the PowerPC builder.

https://github.com/macports/macports-ports/commits/master

According to the above API reference, we're supposed to be able to attach multiple statuses (up to 1000) to each commit, and I see you've got a comment in the master.cfg that buildbot 0.8.14 supports the context parameter, and of course 0.8.12 doesn't, and 0.8.14 was never released, so this is terrible.

Shall we backport these PRs to our buildbot 0.8.12 port?

Add context field to GitHubStatus updates
https://github.com/buildbot/buildbot/pull/1721

Include 'context' in the log message for 'GitHubStatusPush'
https://github.com/buildbot/buildbot/pull/3773


Reply | Threaded
Open this post in threaded view
|

Re: [macports-infrastructure] 02/02: buildbot: Update status on GitHub after each build

Ryan Schmidt-24

On Mar 12, 2018, at 21:46, Mojca Miklavec wrote:

>> Shall we backport these PRs to our buildbot 0.8.12 port?
>>
>> Add context field to GitHubStatus updates
>> https://github.com/buildbot/buildbot/pull/1721
>>
>> Include 'context' in the log message for 'GitHubStatusPush'
>> https://github.com/buildbot/buildbot/pull/3773
>
> That makes most sense to me.

Ok! That's working. The php-APCu commit is the first one that has multiple statuses.

Maybe we want to exclude some of them...

Reply | Threaded
Open this post in threaded view
|

Re: [macports-infrastructure] 02/02: buildbot: Update status on GitHub after each build

Mojca Miklavec-2
On 13 March 2018 at 05:07, Ryan Schmidt wrote:

> On Mar 12, 2018, at 21:46, Mojca Miklavec wrote:
>
>>> Shall we backport these PRs to our buildbot 0.8.12 port?
>>>
>>> Add context field to GitHubStatus updates
>>> https://github.com/buildbot/buildbot/pull/1721
>>>
>>> Include 'context' in the log message for 'GitHubStatusPush'
>>> https://github.com/buildbot/buildbot/pull/3773
>>
>> That makes most sense to me.
>
> Ok! That's working. The php-APCu commit is the first one that has multiple statuses.

Awesome :) :) :)
Thanks a lot.

> Maybe we want to exclude some of them...

I would probably only keep the port watchers.

Mojca
Reply | Threaded
Open this post in threaded view
|

Re: [macports-infrastructure] 02/02: buildbot: Update status on GitHub after each build

Rainer Müller-4
In reply to this post by Ryan Schmidt-24
On 2018-03-13 05:07, Ryan Schmidt wrote:

>
> On Mar 12, 2018, at 21:46, Mojca Miklavec wrote:
>
>>> Shall we backport these PRs to our buildbot 0.8.12 port?
>>>
>>> Add context field to GitHubStatus updates
>>> https://github.com/buildbot/buildbot/pull/1721
>>>
>>> Include 'context' in the log message for 'GitHubStatusPush'
>>> https://github.com/buildbot/buildbot/pull/3773
>>
>> That makes most sense to me.
>
> Ok! That's working. The php-APCu commit is the first one that has multiple statuses.
>
> Maybe we want to exclude some of them...

Looks good!

Sorry, I only tested it with one builder on my local set up with
buildbot 0.8.12 and I assumed the context= parameter was just not
configurable, but would always use the builder's name.

I am working on a filter by builder name.

Rainer
Reply | Threaded
Open this post in threaded view
|

Re: [macports-infrastructure] 02/02: buildbot: Update status on GitHub after each build

Rainer Müller-4
On 2018-03-13 11:06, Rainer Müller wrote:
> I am working on a filter by builder name.

This should be covered with this commit:

https://github.com/macports/macports-infrastructure/commit/830b9ff5b4f0af5d7d2598e01f48ff69074e7742

Rainer
Reply | Threaded
Open this post in threaded view
|

Re: [macports-infrastructure] 02/02: buildbot: Update status on GitHub after each build

Ryan Schmidt-24
In reply to this post by Mojca Miklavec-2

On Mar 13, 2018, at 02:43, Mojca Miklavec wrote:

> On 13 March 2018 at 05:07, Ryan Schmidt wrote:
>> On Mar 12, 2018, at 21:46, Mojca Miklavec wrote:
>>
>>>> Shall we backport these PRs to our buildbot 0.8.12 port?
>>>>
>>>> Add context field to GitHubStatus updates
>>>> https://github.com/buildbot/buildbot/pull/1721
>>>>
>>>> Include 'context' in the log message for 'GitHubStatusPush'
>>>> https://github.com/buildbot/buildbot/pull/3773
>>>
>>> That makes most sense to me.
>>
>> Ok! That's working. The php-APCu commit is the first one that has multiple statuses.
>
> Awesome :) :) :)
> Thanks a lot.
>
>> Maybe we want to exclude some of them...
>
> I would probably only keep the port watchers.

Surely we want to keep port builders, not port watchers?

Reply | Threaded
Open this post in threaded view
|

Re: [macports-infrastructure] 02/02: buildbot: Update status on GitHub after each build

Rainer Müller-4
On 2018-03-13 13:27, Ryan Schmidt wrote:

>
> On Mar 13, 2018, at 02:43, Mojca Miklavec wrote:
>
>> On 13 March 2018 at 05:07, Ryan Schmidt wrote:
>>> On Mar 12, 2018, at 21:46, Mojca Miklavec wrote:
>>>
>>>>> Shall we backport these PRs to our buildbot 0.8.12 port?
>>>>>
>>>>> Add context field to GitHubStatus updates
>>>>> https://github.com/buildbot/buildbot/pull/1721
>>>>>
>>>>> Include 'context' in the log message for 'GitHubStatusPush'
>>>>> https://github.com/buildbot/buildbot/pull/3773
>>>>
>>>> That makes most sense to me.
>>>
>>> Ok! That's working. The php-APCu commit is the first one that has multiple statuses.
>>
>> Awesome :) :) :)
>> Thanks a lot.
>>
>>> Maybe we want to exclude some of them...
>>
>> I would probably only keep the port watchers.
>
> Surely we want to keep port builders, not port watchers?

Hm, but we run multiple builders per commit. I guess the context would
then have to include the portname or the results would be overwritten again.

Rainer
Reply | Threaded
Open this post in threaded view
|

Re: [macports-infrastructure] 02/02: buildbot: Update status on GitHub after each build

Ryan Schmidt-24

On Mar 13, 2018, at 07:50, Rainer Müller wrote:

> On 2018-03-13 13:27, Ryan Schmidt wrote:
>>
>> On Mar 13, 2018, at 02:43, Mojca Miklavec wrote:
>>
>>> On 13 March 2018 at 05:07, Ryan Schmidt wrote:
>>>> On Mar 12, 2018, at 21:46, Mojca Miklavec wrote:
>>>>
>>>>>> Shall we backport these PRs to our buildbot 0.8.12 port?
>>>>>>
>>>>>> Add context field to GitHubStatus updates
>>>>>> https://github.com/buildbot/buildbot/pull/1721
>>>>>>
>>>>>> Include 'context' in the log message for 'GitHubStatusPush'
>>>>>> https://github.com/buildbot/buildbot/pull/3773
>>>>>
>>>>> That makes most sense to me.
>>>>
>>>> Ok! That's working. The php-APCu commit is the first one that has multiple statuses.
>>>
>>> Awesome :) :) :)
>>> Thanks a lot.
>>>
>>>> Maybe we want to exclude some of them...
>>>
>>> I would probably only keep the port watchers.
>>
>> Surely we want to keep port builders, not port watchers?
>
> Hm, but we run multiple builders per commit.

Oh right. It just seems like if a commit builds just one port, then its annoying to have to click on the portwatcher details, only to have to click another link to get the portbuilder details to see what happened. And if more than one port builds, then once you get to portwatcher and you have multiple logs to click, but there's no indication of which log is for which port.

At the moment, portwatcher can also be marked as failed because of a failed mirroring job, and that might be annoying. Currently, for example, any port depending indirectly on mesa will fail to mirror, because its python26 variant is broken. This will clear itself up once distfiles are mirrored first, before portwatcher.

> I guess the context would
> then have to include the portname or the results would be overwritten again.

That could work. I kind of don't like the tiny little box that GitHub restricts the status information to. It's barely wide enough to contain the context string, and not wide enough to show the build status string. But if we make the context shorter, by removing "ports-" and "-portbuilder" and maybe even "-x86_64" we'd have more room for a port name.

Reply | Threaded
Open this post in threaded view
|

Re: [macports-infrastructure] 02/02: buildbot: Update status on GitHub after each build

Mojca Miklavec-2
On 13 March 2018 at 16:15, Ryan Schmidt wrote:

> On Mar 13, 2018, at 07:50, Rainer Müller wrote:
>> On 2018-03-13 13:27, Ryan Schmidt wrote:
>>> On Mar 13, 2018, at 02:43, Mojca Miklavec wrote:
>>>> On 13 March 2018 at 05:07, Ryan Schmidt wrote:
>>>>>
>>>>> Maybe we want to exclude some of them...
>>>>
>>>> I would probably only keep the port watchers.
>>>
>>> Surely we want to keep port builders, not port watchers?
>>
>> Hm, but we run multiple builders per commit.
>
> Oh right. It just seems like if a commit builds just one port, then its annoying to have to click on the portwatcher details, only to have to click another link to get the portbuilder details to see what happened. And if more than one port builds, then once you get to portwatcher and you have multiple logs to click, but there's no indication of which log is for which port.

We can probably change context and show the port name, we just need to
keep in mind that GitHub might then show hundreds of lines and at some
point we'll no longer be able to see some of them anyway.

> At the moment, portwatcher can also be marked as failed because of a failed mirroring job, and that might be annoying. Currently, for example, any port depending indirectly on mesa will fail to mirror, because its python26 variant is broken. This will clear itself up once distfiles are mirrored first, before portwatcher.

True.

>> I guess the context would
>> then have to include the portname or the results would be overwritten again.
>
> That could work. I kind of don't like the tiny little box that GitHub restricts the status information to. It's barely wide enough to contain the context string, and not wide enough to show the build status string. But if we make the context shorter, by removing "ports-" and "-portbuilder" and maybe even "-x86_64" we'd have more room for a port name.

We could also shorten the "buildbot" label itself.

Mojca