New issue
Advanced search Search tips

Issue 914996 link

Starred by 1 user
Status: Fixed
Owner:
no...@chromium.org
Closed: Dec 20
Cc:
iannu...@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Task

Blocking:
issue 875056



Sign in to add a comment

Update recipes to accept float numbers

Project Member Reported by no...@chromium.org, Dec 13 
Recipe and buildbucket are implemented in Python and thus they accidentally distinguished ints and float property values, although JSON spec does not distinguish them, it has only one type: number. Buildbucket is switching to google.protobuf.Struct format for properties, which erases the difference. Struct supports only doubles.
Thus going forward numeric property values will be delivered to recipes as floats.

This bug is about updating such recipes to convert floats to ints if they accept an integer.

Comment 1 by no...@chromium.org, Dec 13 

bug 914992 (property schema as a proto) is a long term solution for this problem
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 13 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/d22b454d8d59edb260c05b0a2a1fb14d0d713491

commit d22b454d8d59edb260c05b0a2a1fb14d0d713491
Author: Nodir Turakulov <nodir@google.com>
Date: Thu Dec 13 23:30:33 2018

[goma] Allow float "jobs" parameter.

Bug:  914996 
Change-Id: Id4b6db447a5f0f96c3bb9663e625f73a4e14f2e1
Reviewed-on: https://chromium-review.googlesource.com/c/1377180
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/d22b454d8d59edb260c05b0a2a1fb14d0d713491/scripts/slave/recipe_modules/goma/api.py
[modify] https://crrev.com/d22b454d8d59edb260c05b0a2a1fb14d0d713491/scripts/slave/recipe_modules/goma/__init__.py
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 14 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/1c2c711b9c9db96ebb1bef2a36870d662208bee4

commit 1c2c711b9c9db96ebb1bef2a36870d662208bee4
Author: Nodir Turakulov <nodir@google.com>
Date: Fri Dec 14 19:59:58 2018

[v8] accept float shards

See  issue 914996  for context.
Allow shards to be a float.

R=machenbach@chromium.org, sergiyb@chromium.org

Bug:  914996 
Change-Id: Iaff545b877d8d1ffabd730d238af67fb917f1357
Reviewed-on: https://chromium-review.googlesource.com/c/1377185
Reviewed-by: Sergiy Belozorov <sergiyb@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/1c2c711b9c9db96ebb1bef2a36870d662208bee4/scripts/slave/recipe_modules/v8/builders.py
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 14 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/5f7befa0045c2887210b347b66a6e2f2dff4a28a

commit 5f7befa0045c2887210b347b66a6e2f2dff4a28a
Author: Nodir Turakulov <nodir@google.com>
Date: Fri Dec 14 21:22:23 2018

[buildbucket] Log integer properties later

Log integer properties after applying properties from buildbucket configs.

Also just report all properties

Also don't log boolean properties.

TBR=iannucci@chromium.org

Bug:  914996 
Change-Id: I0ebc13a5a60666aaa98e2043d9b7006be6d815ea
Reviewed-on: https://chromium-review.googlesource.com/c/1378788
Commit-Queue: Nodir Turakulov <nodir@chromium.org>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#19570}
[modify] https://crrev.com/5f7befa0045c2887210b347b66a6e2f2dff4a28a/appengine/cr-buildbucket/test/creation_test.py
[modify] https://crrev.com/5f7befa0045c2887210b347b66a6e2f2dff4a28a/appengine/cr-buildbucket/creation.py

Comment 5 by nodir@google.com, Dec 14 

here are some properties seen in prod:

- reviewers.* - b/121042927
- parent_test_spec.* - https://chromium-review.googlesource.com/c/chromium/tools/build/+/1377185
- depends_on.* - b/121042927
- parent_buildnumber - https://chromium-review.googlesource.com/c/chromium/tools/build/+/1378867
- cbb_master_build_id - https://chromium-review.googlesource.com/c/chromiumos/chromite/+/1378905/
- clobber - works the same
- root_solution_revision_timestamp - https://chromium-review.googlesource.com/c/chromium/tools/build/+/1378868/
- worker - https://fuchsia-review.googlesource.com/c/infra/recipes/+/234356
- buildset - b/121042927
- attempt_start_ts - CQ does not read the property, cq appengine app converts to int
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 17 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/a475e97f31e6fbbeee14d5e601f5e9672c8b8ea6

commit a475e97f31e6fbbeee14d5e601f5e9672c8b8ea6
Author: Nodir Turakulov <nodir@google.com>
Date: Mon Dec 17 23:23:08 2018

Make cbb_master_build_id property a string

See comment for the reasoning and the context bug.

Bug:  914996 
Change-Id: I888414de31ff8c6203c22bc33494a44f66c7d3eb
Reviewed-on: https://chromium-review.googlesource.com/c/1378905
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>
Tested-by: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/a475e97f31e6fbbeee14d5e601f5e9672c8b8ea6/lib/request_build.py
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 18 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/92a20048b2445b989db7a4108be3af1c2702eda2

commit 92a20048b2445b989db7a4108be3af1c2702eda2
Author: Nodir Turakulov <nodir@google.com>
Date: Tue Dec 18 17:39:50 2018

Tolerate float root_solution_revision_timestamp

See the context bug for the reasoning.

Bug:  914996 
Change-Id: Id8a5d7ea9b0cdee7e6a0704dde4a9c4d39247eeb
Reviewed-on: https://chromium-review.googlesource.com/c/1378868
Commit-Queue: Nodir Turakulov <nodir@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/92a20048b2445b989db7a4108be3af1c2702eda2/scripts/slave/recipes/chromium_codesearch.py
[modify] https://crrev.com/92a20048b2445b989db7a4108be3af1c2702eda2/scripts/slave/recipes/chromium_codesearch_initiator.expected/missing_timestamp.json
[modify] https://crrev.com/92a20048b2445b989db7a4108be3af1c2702eda2/scripts/slave/recipes/chromium_codesearch_initiator.expected/basic.json
[modify] https://crrev.com/92a20048b2445b989db7a4108be3af1c2702eda2/scripts/slave/recipes/chromium_codesearch_initiator.expected/bad_timestamp.json
[modify] https://crrev.com/92a20048b2445b989db7a4108be3af1c2702eda2/scripts/slave/README.recipes.md
[modify] https://crrev.com/92a20048b2445b989db7a4108be3af1c2702eda2/scripts/slave/recipes/chromium_codesearch_initiator.py
[modify] https://crrev.com/92a20048b2445b989db7a4108be3af1c2702eda2/scripts/slave/recipes/chromium_codesearch_initiator.expected/missing_commit.json
Project Member

Comment 13 by bugdroid1@chromium.org, Dec 18 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/build_limited/scripts/slave/+/98fed12ed3ea09ea33003d2099f462f677f59443

commit 98fed12ed3ea09ea33003d2099f462f677f59443
Author: Nodir Turakulov <nodir@google.com>
Date: Tue Dec 18 18:05:19 2018
Project Member

Comment 14 by bugdroid1@chromium.org, Dec 18 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/09cefa792cf2d046c20c444d23c5e1b2144ca870

commit 09cefa792cf2d046c20c444d23c5e1b2144ca870
Author: Nodir Turakulov <nodir@google.com>
Date: Tue Dec 18 18:24:27 2018

Tolerate float target_bits

See the bug for context

R=sergiyb@chromium.org, tandrii@chromium.org

TBR=sergiyb@chromium.org
Bug:  914996 
Change-Id: Ia625e24b9fe607d73d2bdbabfad673ee9983ac47
Reviewed-on: https://chromium-review.googlesource.com/c/1382793
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/09cefa792cf2d046c20c444d23c5e1b2144ca870/scripts/slave/recipes/v8/archive.py
Project Member

Comment 15 by bugdroid1@chromium.org, Dec 19 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/cb17b504b449c6b048859855981e3f261af90022

commit cb17b504b449c6b048859855981e3f261af90022
Author: Nodir Turakulov <nodir@google.com>
Date: Wed Dec 19 00:56:16 2018

[kitchen] Stop supporting integer properties

kitchen used to support int-float differentiation in property values.
This CL removes the support.

Motivation: it turned out the protobuf library available in GAE env is old
and does not enforce floats. Better to enforce float now than having a time
bomb

Bug:  914996 
Change-Id: I9c83c66a81e1cdf99047d5d28f9151f800d7c856
Reviewed-on: https://chromium-review.googlesource.com/c/1382687
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#19657}
[modify] https://crrev.com/cb17b504b449c6b048859855981e3f261af90022/go/src/infra/tools/kitchen/cook.go
[modify] https://crrev.com/cb17b504b449c6b048859855981e3f261af90022/go/src/infra/tools/kitchen/util.go

Comment 16 by no...@chromium.org, Dec 20

Status: Fixed (was: Assigned)
Project Member

Comment 17 by bugdroid1@chromium.org, Jan 4 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infradata/config/+/afd742c246e3f287e11b9dfc59ad721ae521ce22

commit afd742c246e3f287e11b9dfc59ad721ae521ce22
Author: Nodir Turakulov <nodir@google.com>
Date: Fri Jan 04 19:02:04 2019
►

Sign in to add a comment