parse_textproto doesn't parse text protobufs |
|||
Issue descriptionhttps://luci-logdog.appspot.com/v/?s=infra%2Fswarm%2Fchromium-swarm.appspot.com%2F35a27f775c76f911%2F%2B%2Fsteps%2FUncaught_Exception%2F0%2Flogs%2Fexception%2F0 Traceback (most recent call last): File "/b/s/w/ir/kitchen-checkout/recipes/.recipe_deps/recipe_engine/recipe_engine/run.py", line 316, in _new_run recipe_result = recipe_script.run(api, properties) File "/b/s/w/ir/kitchen-checkout/recipes/.recipe_deps/recipe_engine/recipe_engine/loader.py", line 81, in run self.run_steps, properties, self.PROPERTIES, api=api) File "/b/s/w/ir/kitchen-checkout/recipes/.recipe_deps/recipe_engine/recipe_engine/loader.py", line 592, in invoke_with_properties **additional_args) File "/b/s/w/ir/kitchen-checkout/recipes/.recipe_deps/recipe_engine/recipe_engine/loader.py", line 553, in _invoke_with_properties return callable_obj(*props, **additional_args) File "/b/s/w/ir/kitchen-checkout/recipes/recipes/recipe_roll_tryjob.py", line 90, in RunSteps patches_raw, rietveld, issue, patchset, patch_project) File "/b/s/w/ir/kitchen-checkout/recipes/.recipe_deps/recipe_engine/recipe_engine/recipe_api.py", line 578, in _inner return func(*a, **kw) File "/b/s/w/ir/kitchen-checkout/recipes/recipe_modules/recipe_tryjob/api.py", line 284, in run_tryjob p: self._get_project_config(p) for p in all_projects} File "/b/s/w/ir/kitchen-checkout/recipes/recipe_modules/recipe_tryjob/api.py", line 284, in <dictcomp> p: self._get_project_config(p) for p in all_projects} File "/b/s/w/ir/kitchen-checkout/recipes/.recipe_deps/recipe_engine/recipe_engine/recipe_api.py", line 578, in _inner return func(*a, **kw) File "/b/s/w/ir/kitchen-checkout/recipes/recipe_modules/recipe_tryjob/api.py", line 133, in _get_project_config parsed = self.m.luci_config.parse_textproto(result['content'].split('\n')) File "/b/s/w/ir/kitchen-checkout/recipes/.recipe_deps/recipe_engine/recipe_engine/recipe_api.py", line 578, in _inner return func(*a, **kw) File "/b/s/w/ir/kitchen-checkout/recipes/.recipe_deps/build/scripts/slave/recipe_modules/luci_config/api.py", line 120, in parse_textproto 'Could not understand line: <%s>' % line) # pragma: no cover ValueError: Could not understand line: <{> *deep breath* Please just remove this hacky text protobuf parser and replace it with something sensible. Next time you're thinking about writing your own text protobuf parser using regular expressions, please step away from the keyboard and reconsider your life choices.
,
Apr 20 2017
> As I'm sure you know, the google.protobuf package wasn't available in the recipe engine until after this tryjob was written. I know, but the right move should've been to add it properly as a third_party dependency, not add *even more* technical debt. LUCI is supposed to be the new well-written clean system that replaces the old crufty system. Are you really happy adding this kind of code to it? > I'm going to disable this tryjob for now. Can you also delete this parse_textproto method? Or is it used elsewhere as well?
,
Apr 20 2017
I didn't add this code or implement this recipe, so I can't speak to that. It definitely seems like a hack to me. I believe, as this is an experimental builder, that it was an experiment. This method was only used for parsing recipes.cfg when it was in textproto. Fortunately we recently transitioned recipes.cfg away from textproto (to jsonpb) so these hacks are no longer necessary.
,
Apr 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/e52c84c17136e7bd32aa8315c80d7b58e4ce1c0e commit e52c84c17136e7bd32aa8315c80d7b58e4ce1c0e Author: Robert Iannucci <iannucci@chromium.org> Date: Thu Apr 20 06:24:18 2017 [cq.cfg] disable broken downstream roll recipes They can obviously be re-enabled when the recipe is expected to work. R=dsansome@chromium.org, phajdan.jr@chromium.org Bug: 713517 Change-Id: I96108a781ccf4837869940d515f58f47fa24a4f9 Reviewed-on: https://chromium-review.googlesource.com/482940 Reviewed-by: Dave Sansome <dsansome@chromium.org> Commit-Queue: Robbie Iannucci <iannucci@chromium.org> [modify] https://crrev.com/e52c84c17136e7bd32aa8315c80d7b58e4ce1c0e/infra/config/cq.cfg
,
Apr 20 2017
This is actually a nasty little piece of technical debt which is already paid off so I can delete it without issue :)
,
Apr 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/18e2870ec189160eac7cd2ee3a40120843c44848 commit 18e2870ec189160eac7cd2ee3a40120843c44848 Author: Robert Iannucci <iannucci@chromium.org> Date: Thu Apr 20 07:01:02 2017 [luci_config] parse_textproto from the recipe_module. This was only ever used to 'parse' recipes.cfg, but that file has since moved to JSONPB specifically to avoid this hack (in a different program). Purge it with fire before anyone is tempted to use it for something else. R=martiniss@chromium.org Bug: 713517 Change-Id: I3cee851df5b7cb408b5561fe8a01e65395764b36 Reviewed-on: https://chromium-review.googlesource.com/482942 Reviewed-by: Dave Sansome <dsansome@chromium.org> Commit-Queue: Robbie Iannucci <iannucci@chromium.org> [modify] https://crrev.com/18e2870ec189160eac7cd2ee3a40120843c44848/scripts/slave/recipe_modules/luci_config/example.py [modify] https://crrev.com/18e2870ec189160eac7cd2ee3a40120843c44848/scripts/slave/recipe_modules/luci_config/api.py [delete] https://crrev.com/e52c84c17136e7bd32aa8315c80d7b58e4ce1c0e/scripts/slave/recipe_modules/luci_config/example.expected/protobuf.json
,
Apr 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/58283d7793b7073043d7031f369f048eeb930a64 commit 58283d7793b7073043d7031f369f048eeb930a64 Author: Robert Iannucci <iannucci@chromium.org> Date: Thu Apr 20 07:07:38 2017 [recipe_tryjob] delete usage of parse_textproto. Recipes.cfg has moved to JSON to avoid all of this nastiness so we can delete this thing. R=martiniss@google.com Bug: 713517 Change-Id: I6133b700e31085db3ce2db08e219c1bb7c22b358 Reviewed-on: https://chromium-review.googlesource.com/482476 Reviewed-by: Dave Sansome <dsansome@chromium.org> Commit-Queue: Robbie Iannucci <iannucci@chromium.org> [modify] https://crrev.com/58283d7793b7073043d7031f369f048eeb930a64/recipes/recipe_modules/recipe_tryjob/__init__.py [modify] https://crrev.com/58283d7793b7073043d7031f369f048eeb930a64/recipes/recipe_modules/recipe_tryjob/test_api.py [modify] https://crrev.com/58283d7793b7073043d7031f369f048eeb930a64/recipes/recipe_modules/recipe_tryjob/api.py
,
Apr 20 2017
poof, gone |
|||
►
Sign in to add a comment |
|||
Comment 1 by iannu...@google.com
, Apr 20 2017Labels: -Pri-1 Pri-3
Owner: ----
Status: Available (was: Unconfirmed)