Code review comment for lp://qastaging/~pfalcon/linaro-android-build-tools/getpass

Revision history for this message
Deepti B. Kalakeri (deeptik) wrote :

On Mon, May 21, 2012 at 6:51 PM, Paul Sokolovsky <<email address hidden>
> wrote:

> You have been requested to review the proposed merge of
> lp:~pfalcon/linaro-android-build-tools/getpass into
> lp:linaro-android-build-tools.
>
> For more details, see:
>
> https://code.launchpad.net/~pfalcon/linaro-android-build-tools/getpass/+merge/106628<https://code.launchpad.net/%7Epfalcon/linaro-android-build-tools/getpass/+merge/106628>
>
> These are changes needed to run mangle-jobs script on ci.linaro.orgcomfortably.
>
> Once this is merged, migration to add explicit build expiration to all
> jobs will be:
>
> $ ./mangle-jobs --user=<Jenkins username> build-expiration-set.mangle
> >mangle.diff
> Password: <Jenkins API key>
>
> [review changes to be done in mangle.diff]
>
> $ ./mangle-jobs --user=<Jenkins username> build-expiration-set.mangle
> --really
> Password: <Jenkins API key>
>
>
> See README file for complete info on mangle-jobs.
>
>
> --
>
> https://code.launchpad.net/~pfalcon/linaro-android-build-tools/getpass/+merge/106628<https://code.launchpad.net/%7Epfalcon/linaro-android-build-tools/getpass/+merge/106628>
> You are requested to review the proposed merge of
> lp:~pfalcon/linaro-android-build-tools/getpass into
> lp:linaro-android-build-tools.
>
> === modified file 'utils/mangle-jobs/README'
> --- utils/mangle-jobs/README 2012-03-08 18:07:13 +0000
> +++ utils/mangle-jobs/README 2012-05-21 13:19:18 +0000
> @@ -53,10 +53,14 @@
> if needed.
>
> 5. You are ready to perform en-masse pre-production test now. You should
> have
> -Jenkins username with appropriate permissions and its password (stored in
> -a file) handy.
> +Jenkins username with appropriate permissions and its password or API key
> +(recommended) handy. The API key can found in Jenkins by clicking you
> username
> +in the top right corner, then Configure in menu. Password/API key will be
> +prompted on teh console. If you really need that, you can put the
> credential
> +in the file and refer to it with --passwd-file=<passwd_file> to avoid
> +interactive prompts.
>
> -$ ./mangle-jobs --user=<user> --passwd-file=<passwd_file>
> <your_script.mangle>
> +$ ./mangle-jobs --user=<user> <your_script.mangle>
>
>
I believe we need to run mangle-jobs from the host on which jenkins service
is hosted ?

> This will run your script repeatedly for each job store in Jenkins server
> and
> will show aggregated diff output. Review it carefully to watch for
> anomalities
> @@ -65,7 +69,7 @@
> 6. Once you're absolutely sure that the changes performed by your mangle
> script
> are correct, run it in the production update mode:
>
> -$ ./mangle-jobs --user=<user> --passwd-file=<passwd_file>
> <your_script.mangle> --really
> +$ ./mangle-jobs --user=<user> <your_script.mangle> --really
>
> Mangle script details
> ---------------------
>
> === modified file 'utils/mangle-jobs/mangle-jobs'
> --- utils/mangle-jobs/mangle-jobs 2012-04-26 11:21:06 +0000
> +++ utils/mangle-jobs/mangle-jobs 2012-05-21 13:19:18 +0000
> @@ -21,10 +21,13 @@
> from tempfile import NamedTemporaryFile
> import urllib2
> import optparse
> +import getpass
>
> from lxml.etree import fromstring, tostring
>
>
> +JENKINS_SERVER = "http://localhost:9090/jenkins/"
>

Setting the value of this using the command line parameter makes it more
flexible than setting this in the script.
For example I use port 8080 for my jenkins service and I would need to make
the changes in the script to run it ?

> +
> optparser = optparse.OptionParser(usage="%prog <mangle script>")
> optparser.add_option("--user",
> help="Jenkins username")
> @@ -50,6 +53,9 @@
> password = None
> if options.passwd_file:
> password = open(options.passwd_file).read().strip()
> +elif not options.file:
> + password = getpass.getpass("Password/API Token:")
> +
> auth_headers = {
> 'Authorization': 'Basic %s' % (
> base64.encodestring('%s:%s' % (options.user, password))[:-1],),
> @@ -71,7 +77,7 @@
> if extra_headers:
> headers.update(extra_headers)
> req = urllib2.Request(
> - 'http://localhost:9090/jenkins/' + jenkins_path, data, headers)
> + JENKINS_SERVER + jenkins_path, data, headers)
> resp = urllib2.urlopen(req)
> return resp.read()
>
> @@ -146,7 +152,7 @@
> return bool(re.search(r, job_name)) ^ neg
>
> def process_remote_jenkins():
> - jobs = json.load(urllib2.urlopen('
> http://localhost:9090/jenkins/api/json?tree=jobs[name]'))
> + jobs = json.load(urllib2.urlopen(JENKINS_SERVER +
> 'api/json?tree=jobs[name]'))
> names = [job['name'] for job in jobs['jobs']]
> names = [name for name in names if name == 'blank' or '_' in name]
> limit = options.limit
>
>
>

--
Thanks and Regards,
Deepti
Infrastructure Team Member, Linaro Platform Teams
Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

« Back to merge proposal