Code review comment for lp:~milo/linaro-ci-dashboard/correct-chain-build

Revision history for this message
Milo Casagrande (milo) wrote :

On Fri, Sep 14, 2012 at 1:51 PM, Stevan Radaković
<email address hidden> wrote:
> Review: Needs Information
> This needs some more work here in my opinion.
> You also have a wrong description here. The build was scheduled for the chained loop, it's just that the results of the textfield loop build were not transferred to the android loop build.

Yes, the point is that the values are not used at all at this stage.
Even if it is scheduled, it is not building anything since we are not
doing anything with those results.

> Now the questions remain: shouldn't the dict parameters be mapped into the real android loop fields now?

That was my understanding of the chainability of loops, at least WRT
android textfield loops and android loop.
They write everything in the Android textfield, we transfer that
inside the AndroidLoop and runs from there.

Even if we do not want to map parameters into the field, we need to do
something with the dict that is passed, otherwise it is not used. The
same could be said if we do not want to map things, we can easily drop
AndroidLoop and we do not need the chainability with it (we just move
the logic into the textfield loop).

> What is "user_defined_values"?

The AndroidLoop model is exactly the same model as is in the wiki
description. The point of "user_defined_values" text field is that
when they create a build, they write variables that are not documented

Take a look here:

There are variables that are not part of the Android build service as
we have now (an example MONKEY_RUNNER):
Those should be environment variable used at some stage of the build.
Where: I have no idea.

The idea behind AndroidLoop was to have all the necessary fields (as
described in the wiki) as first class fields in the model, but still
giving the user a way to define the other kind of variables.

> Do they override the other fields in the loop?

They actually can, because there is no check that prevents that. The
same could happen if somebody writes the same key<>value pair twice in
the text field.

What I do here is basically setting up the AndroidLoop model with the
values from the dict: if something in there does not match an
attribute of AndroidLoop, store it in 'user_defined_values'. After
that, the configuration passed into Jenkins is the base64 encoded
string of all the values, also of those in 'user_defined_values' (at
the moment this last part is not happening though).

Milo Casagrande
Infrastructure Engineer <> │ Open source software for ARM SoCs

« Back to merge proposal