Merge lp:~linaro-infrastructure/linaro-ci-dashboard/sync-builds into lp:linaro-ci-dashboard

[Stevan Radaković]

Separate integration loop model from loop model.
Add update functionality.
Add index link on detail page.
Add new sync command for jenkins and CI loop build sync. Also refactor the sync method in jenkinsserver model.

[Stevan Radaković (stevanr)]

One test will fail, but I believe Deepti already fixed that on trunk, don't mind it.

[lp:~linaro-infrastructure/linaro-ci-dashboard/sync-builds]

19. By Stevan Radaković on 2012-07-24

PEP8 fixes.

20. By Stevan Radaković on 2012-07-24

Remove some development-used error re-raising.

21. By Stevan Radaković on 2012-07-24

Add exception rule for creating job on remote server.

[Deepti B. Kalakeri (deeptik)]

Ah! good amount of changes. Thanks for working on this.

some problems, that we need to fix

1) I tried merging your code with the upstream linaro-ci-dashboard, there are lot of merge conflicts, please rebase your changes on top of the latest linaro-ci-dashboard.

2) test_create_integration_job_xml test fails because your changes has renamed the function create_job_xml() to create_integration_job_xml.

3) The jenkins sync command only seems to update the job created using the IntegrationLoop/create url. What about the information of the jobs which are created outsider of the Loop, maybe using jenkins directly ?

4) Also, the values stored in the frontend.loopbuild table should match the actual build numbers on the jenkins. For ex: whenever we press the schedule New build we get a new build_number, this should in turn store job buildnumber in remote_buildnumber if the build job was successful. build_job returns the job details in the form of xml once it is successfully scheduled to run or a JenkinsException on failure. We should probably call get_job_info() and get the status of the job and accordingly the status of the job also needs to be updated on the detail page and in the database instead of hard coding.

review: Needs Fixing (code)

[lp:~linaro-infrastructure/linaro-ci-dashboard/sync-builds]

22. By Stevan Radaković on 2012-07-25

Merge from trunk.

23. By Stevan Radaković on 2012-07-25

Add remote_number functionality to CI loop build.

[Stevan Radaković (stevanr)]

Good points Deepti.

I've fixed everything except 3) as we don't rly wanna care about the job/builds created/scheduled outside of our application. What we have here is kinda of a standalone app without any dependencies from outside CI as suggested by Danilo in his email.

[Deepti B. Kalakeri (deeptik)]

Changes looks good, Except one problem. The loop_build db build_number is incremented by +2 instead of +1. We need to change that.

Well I still feel we should handle the case 3, its important as there might be jenkins which is already running since quite sometime and we thought of using our app now to integrate with the already running jenkins. In that case, the data would not be properly synced. May be we can consider this to be part of future development. But it is worth capturing this in the blog.

review: Needs Fixing (code)

[lp:~linaro-infrastructure/linaro-ci-dashboard/sync-builds]

24. By Stevan Radaković on 2012-07-25

Fix build_number incrementation.

[Deepti B. Kalakeri (deeptik)]

The build_number is always set to 1. I think this is because the save() operation on the build_loop object is called twice and the culprit seems to be in ./dashboard/frontend/models/loop.py schedule_build() function which calls the save twice. The first time when its called the value of the build_number is incremented properly but, the second time the build_number is set to None and hence the value gets reset to 1.
We need only one save() call in the schedule_build() the save() call before the

>> build.remote_number = self.server.schedule_job_build(self.name) can be removed and the one after the call can be retained.

This should fix the problem easily.

review: Needs Fixing (code)

[lp:~linaro-infrastructure/linaro-ci-dashboard/sync-builds]

25. By Stevan Radaković on 2012-07-26

Build number another fix.

[Stevan Radaković (stevanr)]

Hopefully last fix for build number :)
We should be able to call save() as many times we want for the loop_build record without jeopardizing the build_number. Anyway, I changed the conditions a bit in save() method in loop_build, it works now.

[Deepti B. Kalakeri (deeptik)]

I think there needs to be one to one mapping of the build_number and remote_number ?
If a build number holds information of the same remote number then we should not create a new record.
As discussed on IRC we do not have a way to get the remote_number available as part of the build_job() jenkinsapi and needs some more thought on how this needs to be handled.

All the other changes looks good. +1

review: Approve

[Milo Casagrande (milo)]

Hello there,

finally, my review too!

One thing that I noticed in the README: is python-dev really necessary
to run the app? It takes a lot of dev packages into the system...
I just ran the app without installing python-dev, and it looks
running, but I do not know if internally it needs something else,
since I might not be using all of the features.

I remember yesterday you said that we catch exception, but we do
nothing for now. I quickly saw a couple without a TODO note, maybe
adding one there too so that we do not forget about that? I know we
are going to otherwise...

+++ dashboard/frontend/models/integration_loop.py
+ def save(self, *args, **kwargs):
+ server = JenkinsServer.objects.get(id=1)
+ self.server = server

Maybe just:

self.server = JenkinsServer ...

Or is there a particular reason? Id is the DB_ID value and is always 1?

A couple of PEP8 fixes:
dashboard/frontend/views/integration_loop_create_view.py:27:1: E302
expected 2 blank lines, found 1
jenkinsserver/models/jenkins_server.py:103:29: W291 trailing whitespace
jenkinsserver/models/jenkins_server.py:106:48: E225 missing whitespace
around operator
jenkinsserver/models/jenkins_server.py:131:15: E222 multiple spaces
after operator

It is nothing that will block the merge, but might good to fix.
Ciao.

--
Milo Casagrande
Infrastructure Engineer
Linaro.org <www.linaro.org> │ Open source software for ARM SoCs

[Milo Casagrande (milo)]

review: Approve

[lp:~linaro-infrastructure/linaro-ci-dashboard/sync-builds]

26. By Stevan Radaković on 2012-07-26

Fixes from Milo code review.

[Stevan Radaković (stevanr)]

Thanks for the review Milo,

I've fixed everything you suggested except removing python-dev dep which is used for django migrations if I remember correctly.

Merging now...