Code review comment for lp:~javier.collado/utah/bug1165175

Revision history for this message
Javier Collado (javier.collado) wrote :

@Andy

> > === modified file 'utah/client/testcase.py'
> > CONTROL_SCHEMA = {
> > + '$schema': 'http://json-schema.org/draft-04/schema#',
>
> out of curiosity - what does this do?
>

From the documentation:
---
 The "$schema" keyword is both used as a JSON Schema version identifier and the location of a resource which is itself a JSON Schema, which describes any schema written for this particular version.
---

You can find the whole information here:
http://json-schema.org/latest/json-schema-core.html#anchor22

> if you look at our current code. run_as isn't actually being required.

I've talked to Joe about this and it seems that the code has a default value for the case in which the value isn't provided, but certainly according to the old version of the schema, it's required:
---
'run_as': {
    'type': 'string',
    'required': True,
},
---

> Additionally - a lot of people probably don't want to have to declare
> this. ie - do you make it "jenkins", "utah", etc? So to prevent possible
> regressions, I think we should make run_as be optional.

Maybe the reason to make it required in the past was to make explicit the user the should be used for each command.
I guess I can make "run_as" optional and default to "utah", but for now there won't be any regression if it's required. We can discuss this and create another merge proposal to address this.

Some other thing that Joe has reminded me that we should think about, is that the build, setup and cleanup commands are run with the same user as the utah client. One proposal at the time we talked about this, was to create a mini-schema for all the commands so that the command and the user could be specified for every command.

> as per my other comment - this can probably be removed.

The safer approach for now should be follow what it was required in the old schema and discuss making it optional and what default value to use.

« Back to merge proposal