swarming: isolate defaults

swarming: isolate defaults

Add default isolate server host and namespace to the swarming server config.
Use them if new task request does not specify them.

Also add tests for config validation.

R=maruel@chromium.org
BUG=601022
Committed: https://github.com/luci/luci-py/commit/ba3cfcedc52e7b25767cdc6a985e8e37c24cd852

[nodir, 2016-05-03 05:05:59]

The CQ bit was checked by nodir@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-03 05:06:09]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1942143002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1942143002/20001

[commit-bot: I haz the power, 2016-05-03 05:09:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-03 05:09:27]

Dry run: This issue passed the CQ dry run.

[nodir, 2016-05-03 16:35:24]

Description was changed from

==========
swarming: isolate defaults and output

Add default isolate server host and namespace to the swarming server config.
Use them if new task request does not speicify them.

Also add tests for config validation.

R=maruel@chromium.org
BUG=601022
==========

to

==========
swarming: isolate defaults

Add default isolate server host and namespace to the swarming server config.
Use them if new task request does not speicify them.

Also add tests for config validation.

R=maruel@chromium.org
BUG=601022
==========

[nodir, 2016-05-09 22:50:58]

PTAL

[M-A Ruel, 2016-05-10 15:27:41]

Sorry for the delays.

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/hand...
File appengine/swarming/handlers_endpoints.py (right):

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:274: request.properties =
request.properties or swarming_rpcs.TaskProperties()
request.properties cannot be None so this is not needed. If it's None, throw
BadRequest right away.

After you can just do:

request.properties.inputs_ref = request.properties.inputs_ref or
swarming_rpcs.FilesRef()
request.properties.inputs_ref.isolatedserver =
request.properties.inputs_ref.isolatedserver or cfg.isolate.default_server
request.properties.inputs_ref.namespace =
request.properties.inputs_ref.namespace or cfg.isolate.default_namespace

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:275: default_server = 'https://' +
cfg.isolate.default_server_host
Do not prepend https:// here, it should be listed with it in the config, this
permits testing with http://localhost for example.

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:281: elif not
request.properties.outputs_target:
Please remove.

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/prot...
File appengine/swarming/proto/config.proto (right):

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/prot...
appengine/swarming/proto/config.proto:28: // e.g. "isolateserver.appspot.com"
have it include the protocol

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/prot...
appengine/swarming/proto/config.proto:29: optional string default_server_host =
1;
default_server

[nodir, 2016-05-10 18:49:49]

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/hand...
File appengine/swarming/handlers_endpoints.py (right):

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:274: request.properties =
request.properties or swarming_rpcs.TaskProperties()
On 2016/05/10 15:27:40, M-A Ruel wrote:
> request.properties cannot be None so this is not needed. If it's None, throw
> BadRequest right away.
> 
> After you can just do:
> 
> request.properties.inputs_ref = request.properties.inputs_ref or
> swarming_rpcs.FilesRef()
> request.properties.inputs_ref.isolatedserver =
> request.properties.inputs_ref.isolatedserver or cfg.isolate.default_server
> request.properties.inputs_ref.namespace =
> request.properties.inputs_ref.namespace or cfg.isolate.default_namespace

Done.

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:275: default_server = 'https://' +
cfg.isolate.default_server_host
On 2016/05/10 15:27:40, M-A Ruel wrote:
> Do not prepend https:// here, it should be listed with it in the config, this
> permits testing with http://localhost for example.

Done.

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:281: elif not
request.properties.outputs_target:
On 2016/05/10 15:27:40, M-A Ruel wrote:
> Please remove.

Done in patchset 'rebased'

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/prot...
File appengine/swarming/proto/config.proto (right):

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/prot...
appengine/swarming/proto/config.proto:28: // e.g. "isolateserver.appspot.com"
On 2016/05/10 15:27:41, M-A Ruel wrote:
> have it include the protocol

Done.

https://codereview.chromium.org/1942143002/diff/20001/appengine/swarming/prot...
appengine/swarming/proto/config.proto:29: optional string default_server_host =
1;
On 2016/05/10 15:27:40, M-A Ruel wrote:
> default_server

Done.

[M-A Ruel, 2016-05-12 13:47:24]

Description was changed from

==========
swarming: isolate defaults

Add default isolate server host and namespace to the swarming server config.
Use them if new task request does not speicify them.

Also add tests for config validation.

R=maruel@chromium.org
BUG=601022
==========

to

==========
swarming: isolate defaults

Add default isolate server host and namespace to the swarming server config.
Use them if new task request does not specify them.

Also add tests for config validation.

R=maruel@chromium.org
BUG=601022
==========

[M-A Ruel, 2016-05-12 13:48:26]

lgtm

https://codereview.chromium.org/1942143002/diff/80001/appengine/swarming/hand...
File appengine/swarming/handlers_endpoints.py (right):

https://codereview.chromium.org/1942143002/diff/80001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:271: if request.properties is None:
in general we use the short form:

if not request.properties:

[nodir, 2016-05-12 22:00:09]

https://codereview.chromium.org/1942143002/diff/80001/appengine/swarming/hand...
File appengine/swarming/handlers_endpoints.py (right):

https://codereview.chromium.org/1942143002/diff/80001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:271: if request.properties is None:
On 2016/05/12 13:48:25, M-A Ruel wrote:
> in general we use the short form:
> 
> if not request.properties:

Done.

[nodir, 2016-05-12 23:37:46]

Patchset #7 (id:120001) has been deleted

[nodir, 2016-05-12 23:39:55]

The CQ bit was checked by nodir@chromium.org

[nodir, 2016-05-12 23:39:55]

The patchset sent to the CQ was uploaded after l-g-t-m from maruel@chromium.org
Link to the patchset: https://codereview.chromium.org/1942143002/#ps140001
(title: "rebased")

[commit-bot: I haz the power, 2016-05-12 23:39:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1942143002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1942143002/140001

[commit-bot: I haz the power, 2016-05-12 23:42:34]

Description was changed from

==========
swarming: isolate defaults

Add default isolate server host and namespace to the swarming server config.
Use them if new task request does not specify them.

Also add tests for config validation.

R=maruel@chromium.org
BUG=601022
==========

to

==========
swarming: isolate defaults

Add default isolate server host and namespace to the swarming server config.
Use them if new task request does not specify them.

Also add tests for config validation.

R=maruel@chromium.org
BUG=601022

Committed:
https://github.com/luci/luci-py/commit/ba3cfcedc52e7b25767cdc6a985e8e37c24cd852
==========

[commit-bot: I haz the power, 2016-05-12 23:42:35]

Committed patchset #7 (id:140001) as
https://github.com/luci/luci-py/commit/ba3cfcedc52e7b25767cdc6a985e8e37c24cd852