swarming: refactor cipd input

swarming: refactor cipd input

Add cipd server settings that specify cipd server host and cipd client package.

Refactor "packages" field in task properties and manifest to "cipd_input" 
message that contains the packages and cipd settings.

Allow parameters in CipdPackage.package_name. A bot will provide their values.

This CL changes TaskProperties schema in a backward incompatible way, but we never used "packages" datastore 
field so far, so it is not a problem.

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

[nodir, 2016-05-04 19:16:14]

Description was changed from

==========
swarming: add cipd settings to task properties

Add cipd settings that specify cipd server host and cipd client package.

Refactor TaskProperties.packages field to TaskProperties.cipd_input
which a message that contains the packages and cipd settings.

This CL changes TaskProperties schema in a backware incompatible way,
but didn't use "packages" datastore field so far, so it is not a
problem.

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

to

==========
swarming: add cipd settings to task properties

Add cipd settings that specify cipd server host and cipd client package.

Refactor TaskProperties.packages field to TaskProperties.cipd_input
which a message that contains the packages and cipd settings.

Allow parameters CipdPackage.package_name. A bot will provide their values.

This CL changes TaskProperties schema in a backware incompatible way,
but didn't use "packages" datastore field so far, so it is not a
problem.

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

[nodir, 2016-05-04 19:17:22]

Description was changed from

==========
swarming: add cipd settings to task properties

Add cipd settings that specify cipd server host and cipd client package.

Refactor TaskProperties.packages field to TaskProperties.cipd_input
which a message that contains the packages and cipd settings.

Allow parameters CipdPackage.package_name. A bot will provide their values.

This CL changes TaskProperties schema in a backware incompatible way,
but didn't use "packages" datastore field so far, so it is not a
problem.

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

to

==========
swarming: refactor cipd input

Add cipd server settings that specify cipd server host and cipd client package.

Refactor TaskProperties.packages field to TaskProperties.cipd_input
which a message that contains the packages and cipd settings.

Allow parameters in CipdPackage.package_name. A bot will provide their values.

This CL changes TaskProperties schema in a backward incompatible way, but didn't
use "packages" datastore field so far, so it is not a
problem.

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

[nodir, 2016-05-04 19:21:51]

Description was changed from

==========
swarming: refactor cipd input

Add cipd server settings that specify cipd server host and cipd client package.

Refactor TaskProperties.packages field to TaskProperties.cipd_input
which a message that contains the packages and cipd settings.

Allow parameters in CipdPackage.package_name. A bot will provide their values.

This CL changes TaskProperties schema in a backward incompatible way, but didn't
use "packages" datastore field so far, so it is not a
problem.

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

to

==========
swarming: refactor cipd input

Add cipd server settings that specify cipd server host and cipd client package.

Refactor "packages" field in task properties and manifest to "cipd_input" 
message that contains the packages and cipd settings.

Allow parameters in CipdPackage.package_name. A bot will provide their values.

This CL changes TaskProperties schema in a backward incompatible way, but didn't
use "packages" datastore field so far, so it is not a
problem.

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

[nodir, 2016-05-05 20:24:13]

Patchset #2 (id:20001) has been deleted

[nodir, 2016-05-05 20:24:35]

Description was changed from

==========
swarming: refactor cipd input

Add cipd server settings that specify cipd server host and cipd client package.

Refactor "packages" field in task properties and manifest to "cipd_input" 
message that contains the packages and cipd settings.

Allow parameters in CipdPackage.package_name. A bot will provide their values.

This CL changes TaskProperties schema in a backward incompatible way, but didn't
use "packages" datastore field so far, so it is not a
problem.

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

to

==========
swarming: refactor cipd input

Add cipd server settings that specify cipd server host and cipd client package.

Refactor "packages" field in task properties and manifest to "cipd_input" 
message that contains the packages and cipd settings.

Allow parameters in CipdPackage.package_name. A bot will provide their values.

This CL changes TaskProperties schema in a backward incompatible way, but we
never used "packages" datastore 
field so far, so it is not a problem.

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

[nodir, 2016-05-05 20:25:41]

PTAL

[nodir, 2016-05-05 20:25:48]

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

[commit-bot: I haz the power, 2016-05-05 20:25:58]

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

[commit-bot: I haz the power, 2016-05-05 20:32:47]

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

[commit-bot: I haz the power, 2016-05-05 20:32:48]

Dry run: Try jobs failed on following builders:
  Luci-py Presubmit on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Luci-py%20Presubmit/bui...)

[nodir, 2016-05-10 23:07:24]

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

[commit-bot: I haz the power, 2016-05-10 23:07:39]

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

[nodir, 2016-05-10 23:16:46]

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

[commit-bot: I haz the power, 2016-05-10 23:16:57]

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

[nodir, 2016-05-10 23:17:11]

Patchset #3 (id:60001) has been deleted

[nodir, 2016-05-10 23:17:25]

Rebased. PTAL

[commit-bot: I haz the power, 2016-05-10 23:20:27]

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

[commit-bot: I haz the power, 2016-05-10 23:20:28]

Dry run: This issue passed the CQ dry run.

[M-A Ruel, 2016-05-11 14:56:45]

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/cip...
File appengine/swarming/cipd.py (right):

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/cip...
appengine/swarming/cipd.py:22: #
https://chromium.googlesource.com/infra/infra/+/f1072a132c68532b548458392c544...
why not https://chromium.googlesource.com/infra/infra/+/master/build/README.md ?

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/cip...
appengine/swarming/cipd.py:26: # os can be "linux", "mac" or "windows" and arch
can be "386" or "amd64".
I'd like the comment to call out arm, arm6l or arm7l or something about arm64

not a fan of 'linux' but PARAM_OS_VER is fine to disambiguate.

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/ser...
File appengine/swarming/server/task_request.py (right):

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/ser...
appengine/swarming/server/task_request.py:296: class CipdInput(ndb.Model):
Why not CipdInputs and cipd_inputs? It's can be more than one package.

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/swa...
File appengine/swarming/swarming_rpcs.py (right):

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/swa...
appengine/swarming/swarming_rpcs.py:105: settings = messages.BytesField(1)
why not expand it? I'd prefer that to encoded protobuf.

[nodir, 2016-05-11 15:40:58]

Patchset #5 (id:120001) has been deleted

[nodir, 2016-05-12 01:49:45]

Patchset #5 (id:140001) has been deleted

[nodir, 2016-05-12 01:53:22]

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/cip...
File appengine/swarming/cipd.py (right):

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/cip...
appengine/swarming/cipd.py:22: #
https://chromium.googlesource.com/infra/infra/+/f1072a132c68532b548458392c544...
On 2016/05/11 14:56:44, M-A Ruel wrote:
> why not https://chromium.googlesource.com/infra/infra/+/master/build/README.md
?

I'd like to avoid broken links in case README moves. If someone wants a fresh
version, replacing the hash with master is trivial.

Note that the sentence is still correct: it is that version of the document that
inspired this.

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/cip...
appengine/swarming/cipd.py:26: # os can be "linux", "mac" or "windows" and arch
can be "386" or "amd64".
On 2016/05/11 14:56:44, M-A Ruel wrote:
> I'd like the comment to call out arm, arm6l or arm7l or something about arm64

I've added etc because I cannot be more specific here because
- ATM we don't have arm cipd packages and no document says about arm
- someone needs to investigate this. https://github.com/golang/go/wiki/GoArm
says a go binary can be compiled to different arm architectures, but on the
other hand, I've just compiled a go binary without specifying GOARM value. I
don't know if the binary I got can run on any arm architecture, or just only one
of them. I don't know about arm enough to decide this.
 
> not a fan of 'linux' but PARAM_OS_VER is fine to disambiguate.

yeah, these enum values are dictated by cipd, not this CL. I can't change them.

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/ser...
File appengine/swarming/server/task_request.py (right):

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/ser...
appengine/swarming/server/task_request.py:296: class CipdInput(ndb.Model):
On 2016/05/11 14:56:45, M-A Ruel wrote:
> Why not CipdInputs and cipd_inputs? It's can be more than one package.

Because word "input" does not necessarily mean singular. Input of a function is
its arguments, even if there are many of them.
http://www.dictionary.com/browse/input says input is "data to be entered into a
computer for processing" and "data" is in fact plural.

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/swa...
File appengine/swarming/swarming_rpcs.py (right):

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/swa...
appengine/swarming/swarming_rpcs.py:105: settings = messages.BytesField(1)
On 2016/05/11 14:56:45, M-A Ruel wrote:
> why not expand it? I'd prefer that to encoded protobuf.

Done. This lead to rethinking of how I embed cipd server url and cipd client
package in task properties, which in turn lead to overall simplification. I no
longer embed CipdSettings message in TaskProperties, but instead TaskProperties
has "server" and "client_package" fields. In the new task request handler, these
values are defaulted to the values defined in the server config, just like
isolate defaults. Now ndb models, rpc models, message conversion and all tests
do not deal with proto. PTAL

[nodir, 2016-05-12 01:53:30]

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

[commit-bot: I haz the power, 2016-05-12 01:53:36]

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

[commit-bot: I haz the power, 2016-05-12 01:57:04]

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

[commit-bot: I haz the power, 2016-05-12 01:57:05]

Dry run: This issue passed the CQ dry run.

[M-A Ruel, 2016-05-12 13:49:45]

lgtm

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/cip...
File appengine/swarming/cipd.py (right):

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/cip...
appengine/swarming/cipd.py:26: # os can be "linux", "mac" or "windows" and arch
can be "386" or "amd64".
On 2016/05/12 01:53:22, nodir wrote:
> On 2016/05/11 14:56:44, M-A Ruel wrote:
> > I'd like the comment to call out arm, arm6l or arm7l or something about
arm64
> 
> I've added etc because I cannot be more specific here because
> - ATM we don't have arm cipd packages and no document says about arm
> - someone needs to investigate this. https://github.com/golang/go/wiki/GoArm
> says a go binary can be compiled to different arm architectures, but on the
> other hand, I've just compiled a go binary without specifying GOARM value. I
> don't know if the binary I got can run on any arm architecture, or just only
one
> of them. I don't know about arm enough to decide this.

I cross compile from Ubuntu, scp to my Raspberry Pi 1 and run it there all the
time with no problem.
https://github.com/maruel/dotstar/blob/master/Makefile#L20
It hardly can get any simpler. :)

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/ser...
File appengine/swarming/server/task_request.py (right):

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/ser...
appengine/swarming/server/task_request.py:296: class CipdInput(ndb.Model):
On 2016/05/12 01:53:22, nodir wrote:
> On 2016/05/11 14:56:45, M-A Ruel wrote:
> > Why not CipdInputs and cipd_inputs? It's can be more than one package.
> 
> Because word "input" does not necessarily mean singular. Input of a function
is
> its arguments, even if there are many of them.
> http://www.dictionary.com/browse/input says input is "data to be entered into
a
> computer for processing" and "data" is in fact plural.

Oh, I had never realized. I wish I knew with inputs_ref. :/

https://codereview.chromium.org/1946253003/diff/160001/appengine/swarming/han...
File appengine/swarming/handlers_bot.py (right):

https://codereview.chromium.org/1946253003/diff/160001/appengine/swarming/han...
appengine/swarming/handlers_bot.py:387: 'client_package': (
() not needed

https://codereview.chromium.org/1946253003/diff/160001/appengine/swarming/swa...
File appengine/swarming/swarming_rpcs.py (right):

https://codereview.chromium.org/1946253003/diff/160001/appengine/swarming/swa...
appengine/swarming/swarming_rpcs.py:117: cipd_input =
messages.MessageField(CipdInput, 10)
For the record, I'm only allowing it because there was no known use of item #10,
this is not good hygiene but we can get away with it now since we never used the
protobuf format.

[nodir, 2016-05-12 18:00:28]

https://codereview.chromium.org/1946253003/diff/160001/appengine/swarming/swa...
File appengine/swarming/swarming_rpcs.py (right):

https://codereview.chromium.org/1946253003/diff/160001/appengine/swarming/swa...
appengine/swarming/swarming_rpcs.py:117: cipd_input =
messages.MessageField(CipdInput, 10)
On 2016/05/12 13:49:45, M-A Ruel wrote:
> For the record, I'm only allowing it because there was no known use of item
#10,
> this is not good hygiene but we can get away with it now since we never used
the
> protobuf format.

do you prefer to renumber all fields? I am fine with that

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

https://codereview.chromium.org/1946253003/diff/160001/appengine/swarming/swa...
File appengine/swarming/swarming_rpcs.py (right):

https://codereview.chromium.org/1946253003/diff/160001/appengine/swarming/swa...
appengine/swarming/swarming_rpcs.py:117: cipd_input =
messages.MessageField(CipdInput, 10)
On 2016/05/12 18:00:28, nodir wrote:
> On 2016/05/12 13:49:45, M-A Ruel wrote:
> > For the record, I'm only allowing it because there was no known use of item
> #10,
> > this is not good hygiene but we can get away with it now since we never used
> the
> > protobuf format.
> 
> do you prefer to renumber all fields? I am fine with that

No, renumbering is also bad protobuf hygiene.

We just get away with it this time. :)

[nodir, 2016-05-12 21:57:17]

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

[commit-bot: I haz the power, 2016-05-12 21:57:29]

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

[commit-bot: I haz the power, 2016-05-12 22:00:59]

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

[commit-bot: I haz the power, 2016-05-12 22:01:00]

Dry run: This issue passed the CQ dry run.

[nodir, 2016-05-14 01:35:15]

The CQ bit was checked by nodir@chromium.org

[nodir, 2016-05-14 01:35:16]

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/1946253003/#ps180001
(title: "arm")

[commit-bot: I haz the power, 2016-05-14 01:35:22]

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

[commit-bot: I haz the power, 2016-05-14 01:37:31]

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

[commit-bot: I haz the power, 2016-05-14 01:37:32]

Try jobs failed on following builders:
  Luci-py Presubmit on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Luci-py%20Presubmit/bui...)

[nodir, 2016-05-14 01:51:03]

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/cip...
File appengine/swarming/cipd.py (right):

https://codereview.chromium.org/1946253003/diff/100001/appengine/swarming/cip...
appengine/swarming/cipd.py:26: # os can be "linux", "mac" or "windows" and arch
can be "386" or "amd64".
On 2016/05/12 13:49:45, M-A Ruel wrote:
> On 2016/05/12 01:53:22, nodir wrote:
> > On 2016/05/11 14:56:44, M-A Ruel wrote:
> > > I'd like the comment to call out arm, arm6l or arm7l or something about
> arm64
> > 
> > I've added etc because I cannot be more specific here because
> > - ATM we don't have arm cipd packages and no document says about arm
> > - someone needs to investigate this. https://github.com/golang/go/wiki/GoArm
> > says a go binary can be compiled to different arm architectures, but on the
> > other hand, I've just compiled a go binary without specifying GOARM value. I
> > don't know if the binary I got can run on any arm architecture, or just only
> one
> > of them. I don't know about arm enough to decide this.
> 
> I cross compile from Ubuntu, scp to my Raspberry Pi 1 and run it there all the
> time with no problem.
> https://github.com/maruel/dotstar/blob/master/Makefile#L20
> It hardly can get any simpler. :)

Done.

[nodir, 2016-05-14 01:53:08]

Patchset #7 (id:200001) has been deleted

[nodir, 2016-05-14 01:53:19]

The CQ bit was checked by nodir@chromium.org

[nodir, 2016-05-14 01:53:19]

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/1946253003/#ps220001
(title: "rebased")

[commit-bot: I haz the power, 2016-05-14 01:53:26]

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

[commit-bot: I haz the power, 2016-05-14 01:56:18]

Description was changed from

==========
swarming: refactor cipd input

Add cipd server settings that specify cipd server host and cipd client package.

Refactor "packages" field in task properties and manifest to "cipd_input" 
message that contains the packages and cipd settings.

Allow parameters in CipdPackage.package_name. A bot will provide their values.

This CL changes TaskProperties schema in a backward incompatible way, but we
never used "packages" datastore 
field so far, so it is not a problem.

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

to

==========
swarming: refactor cipd input

Add cipd server settings that specify cipd server host and cipd client package.

Refactor "packages" field in task properties and manifest to "cipd_input" 
message that contains the packages and cipd settings.

Allow parameters in CipdPackage.package_name. A bot will provide their values.

This CL changes TaskProperties schema in a backward incompatible way, but we
never used "packages" datastore 
field so far, so it is not a problem.

R=maruel@chromium.org
BUG=601022

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

[commit-bot: I haz the power, 2016-05-14 01:56:20]

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