-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add support to trigger a specific builder. Auto-open url on request. #8
Conversation
| @@ -3,6 +3,7 @@ | |||
| import treq | |||
| import twisted | |||
|
|
|||
| BASE_URL = 'http://buildbot.twistedmatrix.com' | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why? :)
are twisted developers watched by intelligence agencies ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming no, but TLS is better than no TLS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course. Everyone is watched by intelligence agencies at all times. See for example https://glyph.twistedmatrix.com/2015/01/security-as-stencil.html :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should note that we have recently made the TLS certificate for buildbot.twistedmatrix.com valid and correct.
|
Does this conflict heavily with #2? |
|
Please take a look at latest changes. I think that #2 is on the wrong path.... force-build is a big hack. Instead we should enable support for buildbot try Rather than improving force-build we can spend effort improving the buildbot try tool. I have setup and use I am now trying to learn about Twisted infrastructure and buildbot config so that I can implement twisted-infra/twisted-buildbot-configuration#7 and also work at implementing buildbot try for Twisted |
One problem with |
so maybe we should submit a patch to buildbot try server to configure it so that it does not allow patches... as it already had the capabilities to trigger builds, set properties and wait for builds to end. and if we want to develop a force-build tool which will work with arbitrary buildbot installation, maybe we should develop it as part of the general buildbot project and not in twisted-dev-tools project. but I really like the patch feature of buildbot try. For security audit, the patch is logged by the builder and we can allow only devs with commit right to trigger buildot try schedulers. for new contributors we can set up a travis-ci build, just to make sure new contributors get some feedback from tests and only ask a review after all tests pass. |
|
My understanding of |
It runs a revision + a patch ... but you can also ask to run a revision without a patch. I was trying to state my point that buildbot try can already do what we are trying to achieve with the force-build hack. builbot try is already generic to work with any other buildbot configuration and it can read settings from a config file. I feel that with force-build we are just re-inventing the wheel. Also, in case buildbot try does not have feature X, rather than extending the force-build script as in #2 we should rather contribute directly to buildbot project. Anyway... back to this review. Please add your comment whether your accept or reject this patch. Thanks! |
|
@adiroiban - is this patch still relevant? |
|
I think that a tool which will allow triggering a specific builder with a specific tests is still useful. But rather than hacking this script I prefer to go with a PB try scheduler. If there is will, I would be more than happy to get a try scheduler for builbot. I am not planning to continue to work on this hack for force builds. |
Problem
The current force-build tools is good but could be improved.
While working on a specific problem / platform it would help a lot to be able to run only a specific builder.
Ex. only a windows builder when working on some windows stuff... only twistedchecker or documentation when cleaning docstrings.
Also, after each run, I have to manually open the browser at the specified url... this can also be improved by opening the default browser.
Changes
This is build on top of #7 as otherwise I was not able to install the package for testing and development.
I have remove the duplicated docstring from bin/force-build and twisted_tools/scripts/forcebuild.py
A
-uflag was added to trigger unsupported builds.I was not able to trigger a builder using the GET interface based on https://buildbot.twistedmatrix.com/builders/BUILDER/force resource. It works with POST. Maybe GET is not supported by buildbot and GET support is only a hack for Twisted... I don't know.
This is why unsupported builder need the extra
-uargument.-oflag was added to automatically open the browser.I think that _getURLForBranch should be private so I renamed it.
To help with testing I have introduce the get_method argument. I don't know how else to do it.
I tried to do my best to improve the tools.
I think that this is an improvement over the current state without any regressions.
Also, in the future I would like to replace this simple GET trigger with a proper
buildot trywhich would allow running patches without a commit. This would be much better for TDD.How to test
Trigger all supported builder ... normal business
Trigger all unsupported builder
Trigger a specific supported builder
Trigger unsupported builder and auto open the builder page
Please let a look and let me know what is wrong this current patch.
Thanks!