Code review comment for lp:~milo/linaro-ci-dashboard/android-build-model-refactor

On Wed, Aug 15, 2012 at 12:50 PM, Stevan Radaković
<email address hidden> wrote:
>
> Ok sorry I didn't know that it's representing a job config since it's called the JenkinsConfig and it's part of the jenkinsserver APP, so I realized it's representing general Jenkins server configuration.

Our bad on choosing that name I guess... but as gesha explained, is a
parameter necessary for the the Android Build job, and is called
CONFIG inside Jenkins.

> With that thing cleared up, I would like us to remember general guidelines for the CI Loops development presented by Danilo couple of weeks ago. The point was that we have as small amount of dependency on jenkins in our frontend app as possible.

Yep, that is right.

> In my opinion this new dependency is redundant since we must update the jenkins job each time the loop is created or updated anyway, so why keep it in the DB when we must do it on-the-fly anyway.

From what I saw from the Android builds, they are updated seldomly.
Once a loop is created and doesn't explode, it is left like that.
The on-the-fly option was considered, but since jobs are not updated
that much, it made more sense to store that parameter somewhere and
retrieve it. I think it is more of a tradeoff between taking up a
little bit more space, than starting to create base64 encoded strings
for all the builds on the fly.

> If we rly want to keep the job config anyway, I'd suggest linking it with loop the other way around, by putting loop_id as foreign key in JenkinsConfig (which should be renamed anyway if we decide to keep it) and adding unique constraint, so it's one-on-one relationship. Of course, all the updates to the jenkins config table should then be from the jenkinsserver up, and in no case from the frontend.

Name will be changed, for sure.
Good point about reverting the direction of the relationship. For the
constraint I was considering it, but didn't add it since I wanted to
see if we needed other fields in there that could have been unique
together too.

Another thing, since this is only related to Android, could be to
extend the android_loop model and store the base64 encoded "CONFIG" in
there.

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