Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notify github pull requests #131

Merged
merged 3 commits into from
Dec 2, 2015
Merged

Notify github pull requests #131

merged 3 commits into from
Dec 2, 2015

Conversation

gforcada
Copy link
Member

@gforcada gforcada commented Nov 8, 2015

Use the pre-script to notify the given pull request(s) that a jenkins
job has been started.

Do the same after the jenkins job has been running to let github
know if the run was successful or not.

Another building block to fully automate testing jenkins jobs for pull
requests.

Some caveats:

  • it needs to be tested
  • @tisto @svx it needs pygithub latest release on jenkins slaves, any objection to install it globally?
  • there's no error handling at all, should a huge try...except be enough in this case (so, if anything fails still continue with it, or do not fail because github is unreachable/misbehaving)?

gforcada added a commit to plone/plone.jenkins_node that referenced this pull request Nov 8, 2015
Needed so that Jenkins can notify GitHub about job status.

See:
plone/jenkins.plone.org#131
@tisto
Copy link
Member

tisto commented Nov 9, 2015

@gforcada awesome! I don't have any objections to install pygithub globally. Regarding the error handling, I would prefer specific try/excepts that can tell us exactly what went wrong.

@jensens
Copy link
Member

jensens commented Nov 11, 2015

this is great!

@gforcada
Copy link
Member Author

Ignore any status notification, I'm using this pull request to test itself on jenkins :-)

@gforcada gforcada force-pushed the notify-github-pull-requests branch from 8d036e8 to 215d922 Compare November 23, 2015 09:09
@gforcada
Copy link
Member Author

@plone/testing-team could you review this?

The upside is that now we are checking that everything works as expected, and if not a clear message is given.

The downside is that it PostBuildScript Jenkins Plugin does not have the result of the job as environment variable, thus making it totally useless for us (i.e. we can not report back to github...)

There is another plugin Global Post Script which does know how the job ended, but alas, you can not import system-wide python packages (so no pygithub)... we could try to do that with plain URL calls...

Any idea of another plugin that could fit in here?

We can always create a mr.roboto view that handles that for us, the Global Post Script plugin should call it with all the needed information (pull requests + build status + build URL)...

@tisto
Copy link
Member

tisto commented Nov 23, 2015

Can't we just wrap the post-notification into a buildout script that runs on the jenkins nodes? We should not invest too much into evaluating plugins in my opinion. When we move to the workflow-plugin jobs, this might become useless.

@gforcada
Copy link
Member Author

You mean to create a script on buildout.coredev that gets called after running the tests, it inspects them and then reports back to github?

That could work, sure, we can pass everything via environment variables, but we need to get the test results...

It doubles as typos/errors reporting tool whenever someone makes a mistake
when creating a pull request job.
@gforcada gforcada force-pushed the notify-github-pull-requests branch 2 times, most recently from 58a188e to 217cfcb Compare December 1, 2015 22:09
@gforcada gforcada changed the title WIP: Notify github pull requests Notify github pull requests Dec 1, 2015
@gforcada
Copy link
Member Author

gforcada commented Dec 1, 2015

@tisto I just took a simpler approach (I did not like to have a script on Jenkins to report that a job was started and another on buildout.coredev to report that it finished).

Now both scripts are on Jenkins. The only thing I'm not 100% happy with is how I account for job failure... is there any other nicer way to gather that information? If not we can merge and deploy.

The only remaining part will be to start jobs automatically, which we can revive on mr.roboto...

@tisto
Copy link
Member

tisto commented Dec 2, 2015

Let's give it a try!

tisto added a commit that referenced this pull request Dec 2, 2015
@tisto tisto merged commit 15b454c into master Dec 2, 2015
@tisto tisto removed the in progress label Dec 2, 2015
@tisto tisto deleted the notify-github-pull-requests branch December 2, 2015 18:17
@gforcada
Copy link
Member Author

gforcada commented Dec 2, 2015

Reporting works: plone/Products.CMFPlone#1271

pr-report

@gforcada
Copy link
Member Author

gforcada commented Dec 2, 2015

@tisto btw. I had to reconfigure xvfb... maybe due to the plugin updates?

@tisto
Copy link
Member

tisto commented Dec 2, 2015

I just did the same. :) Yes, I guess so...

@tisto
Copy link
Member

tisto commented Dec 2, 2015

@gforcada AWESOME!!!

@davisagli
Copy link
Member

🍹

@gforcada
Copy link
Member Author

gforcada commented Dec 2, 2015

Works: plone/Products.CMFPlone#1271

@jensens
Copy link
Member

jensens commented Dec 2, 2015

wow, that's really great work! and it will make work with PRs much easier. thx a lot

@gforcada
Copy link
Member Author

gforcada commented Dec 3, 2015

@jensens that's the idea yes, now we only need to recover the pullrequest commit hook from mr.roboto and all pull requests will be fully automated :-)

@jensens
Copy link
Member

jensens commented Dec 3, 2015

@gforcada i'am not sure if it is ok load-wise to test every push on a PR branch? We may need more executors then.

@tisto
Copy link
Member

tisto commented Dec 3, 2015

@jensens I think we could handle that. Either by adding more nodes or restricting the number of parallel pull request builds.

@gforcada
Copy link
Member Author

gforcada commented Dec 3, 2015

@jensens if it takes more time, so be it, although it usually is not that busy and the amount of pull requests per day is not that high, only during sprints/release time is when jenkins is at full capacity...

And for releases we need to not trigger jenkins on zest.releaser commits (there's an issue for that already).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants