Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(582)

Issue 2998523002: Add deployment manifest proto to recipe_engine. (Closed)

Created:
3 years, 4 months ago by iannucci
Modified:
3 years, 4 months ago
CC:
chromium-reviews, infra-reviews+recipes-py_chromium.org
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Add source manifest proto to recipe_engine. This adds the initial form of the "source manifest" proto which will allow recipe_engine to communicate structured data about stuff-on-disk-that-we-got-from-remote-servers to LUCI services like Milo, and will also give us a better option than 'got_revision' properties for exporting this data from recipes (i.e. we'll be able to export all the data and then query/read it instead of hard-coding a few hacks to export specific repos). This only covers git, cipd and isolated for now, but could be expanded to other data sources, if needed. We don't currently anticipate a need for that, but the proto is hopefully obviously extensible for this. Clients consuming this proto (like Milo) may have specialized support when comparing two manifest (e.g. two entries sharing the same git repo may render as a git log), but it should also be somewhat easy to show a useful generic diff when comparing two Manifest protos, even when the deployment type changes (i.e. local path "foo" used to be a git checkout, but is now multiple CIPD packages). R=dnj@chromium.org, hinoka@chromium.org, machenbach@chromium.org BUG=468053 Review-Url: https://codereview.chromium.org/2998523002 Committed: https://github.com/luci/recipes-py/commit/e92def4f3c06e6dc65c8d69f6bbb378ccba92546

Patch Set 1 #

Patch Set 2 : source manifest #

Patch Set 3 : Rename some stuff #

Total comments: 2

Patch Set 4 : fix stuff #

Total comments: 5

Patch Set 5 : fix nits #

Patch Set 6 : typo #

Total comments: 6

Patch Set 7 : directories #

Patch Set 8 : comment on fetch_url #

Unified diffs Side-by-side diffs Delta from patch set Stats (+490 lines, -0 lines) Patch
A recipe_engine/source_manifest.proto View 1 2 3 4 5 6 7 1 chunk +128 lines, -0 lines 0 comments Download
A recipe_engine/source_manifest_pb2.py View 1 2 3 4 5 6 1 chunk +362 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (7 generated)
iannucci
3 years, 4 months ago (2017-08-05 00:56:31 UTC) #1
Vadim Sh.
https://codereview.chromium.org/2998523002/diff/40001/recipe_engine/source_manifest.proto File recipe_engine/source_manifest.proto (right): https://codereview.chromium.org/2998523002/diff/40001/recipe_engine/source_manifest.proto#newcode98 recipe_engine/source_manifest.proto:98: bytes hash = 2; 3
3 years, 4 months ago (2017-08-05 01:29:13 UTC) #4
Michael Achenbach
LGTM with nits on the git and isolate stuff. Don't know much about CIPD. Q: ...
3 years, 4 months ago (2017-08-07 07:01:38 UTC) #5
iannucci
On 2017/08/07 07:01:38, Michael Achenbach wrote: > LGTM with nits on the git and isolate ...
3 years, 4 months ago (2017-08-07 18:59:58 UTC) #6
iannucci
PTAL https://codereview.chromium.org/2998523002/diff/40001/recipe_engine/source_manifest.proto File recipe_engine/source_manifest.proto (right): https://codereview.chromium.org/2998523002/diff/40001/recipe_engine/source_manifest.proto#newcode98 recipe_engine/source_manifest.proto:98: bytes hash = 2; On 2017/08/05 01:29:13, Vadim ...
3 years, 4 months ago (2017-08-07 19:02:28 UTC) #7
iannucci
FYI, still looking for a review from dnj, hinoka and/or vadimsh :)
3 years, 4 months ago (2017-08-08 00:45:23 UTC) #8
Michael Achenbach
https://codereview.chromium.org/2998523002/diff/60001/recipe_engine/source_manifest.proto File recipe_engine/source_manifest.proto (right): https://codereview.chromium.org/2998523002/diff/60001/recipe_engine/source_manifest.proto#newcode27 recipe_engine/source_manifest.proto:27: // version 0 is the only version. Version will ...
3 years, 4 months ago (2017-08-08 06:25:06 UTC) #9
Vadim Sh.
lgtm with a bunch of questions: 1. Is 'cipd_package_pattern' actually easy to get? 2. How ...
3 years, 4 months ago (2017-08-08 19:19:14 UTC) #10
Ryan Tseng
lgtm https://codereview.chromium.org/2998523002/diff/100001/recipe_engine/source_manifest.proto File recipe_engine/source_manifest.proto (right): https://codereview.chromium.org/2998523002/diff/100001/recipe_engine/source_manifest.proto#newcode1 recipe_engine/source_manifest.proto:1: // Copyright 2016 The LUCI Authors. All rights ...
3 years, 4 months ago (2017-08-08 19:55:28 UTC) #12
iannucci
On 2017/08/08 19:19:14, Vadim Sh. wrote: > lgtm with a bunch of questions: > > ...
3 years, 4 months ago (2017-08-08 20:00:51 UTC) #13
iannucci
https://codereview.chromium.org/2998523002/diff/100001/recipe_engine/source_manifest.proto File recipe_engine/source_manifest.proto (right): https://codereview.chromium.org/2998523002/diff/100001/recipe_engine/source_manifest.proto#newcode1 recipe_engine/source_manifest.proto:1: // Copyright 2016 The LUCI Authors. All rights reserved. ...
3 years, 4 months ago (2017-08-08 20:49:18 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2998523002/140001
3 years, 4 months ago (2017-08-08 21:43:46 UTC) #17
commit-bot: I haz the power
3 years, 4 months ago (2017-08-08 21:50:13 UTC) #20
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://github.com/luci/recipes-py/commit/e92def4f3c06e6dc65c8d69f6bbb378ccba...

Powered by Google App Engine
This is Rietveld 408576698