Make output placeholders like json outputs index-able by names. |
|||
Issue descriptionCurrently, if we have multiple placeholders of the same modules (eg: json.output) in a step, the results are positional as a list in the step result (eg: json.output_all https://code.google.com/p/chromium/codesearch#chromium/infra/recipes-py/recipe_modules/json/example.py&l=42). My proposal is to have them indexed by names. This will be more explicit/convenient and less error-prone. For example: result = api.python.inline( 'foo', """ import json import sys with open(sys.argv[1], 'w') as f: f.write(json.dumps([1, 2, 3])) with open(sys.argv[2], 'w') as f: f.write(json.dumps(['x', 'y', %s])) """ % repr(FULLWIDTH_Z), args=[api.json.output(identifier='name1'), api.json.output(identifier='name2')], ) assert result.json.outputs['name1'] == [1, 2, 3] assert result.json.outputs['name2'] == ['x', 'y', FULLWIDTH_Z] Sine a couple of recipes/recipe_modules already use json.output_all, my plan is to do it in three steps: 1. In recipe framework, add support for index, eg: json.outputs[name]. 2. Update recipes/recipe_modules to use name index instead of positional, eg: use json.outputs instead json.output_all 3. In recipe framework, do the clean-up, eg: remove json.output_all.
,
Mar 9 2016
The use case I run into is in https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/compile.py Currently, goma dumps its status into a json.output, while I want to dump some info from ninja into another json.output. I don't think we want to combine both into one.
,
Mar 9 2016
Some internal recipes/recipe_modules also have two json.output as parameters in the command line like this.
,
Mar 9 2016
Yep, I think there are going to be more cases like this soon too (like monitoring library pooping some data to one json file, while the actual script needs to produce some data into another file)
,
Mar 9 2016
Yeah, ok. Makes sense, go for it! :)
,
Mar 22 2016
Now all the changes are ready. Changes on recipe_engine side: https://codereview.chromium.org/1785543004/ https://codereview.chromium.org/1773273003/ Changes on recipes/recipe_moduels side: build/ : https://codereview.chromium.org/1820073003/ build_internal/ : https://chromereviews.googleplex.com/380557013/ depot_tools/ : no change is needed as I have checked.
,
Mar 22 2016
,
Mar 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build.git/+/c155ba71c085f6f299de1e00f6b959d1f9b31819 commit c155ba71c085f6f299de1e00f6b959d1f9b31819 Author: stgao@chromium.org <stgao@chromium.org> Date: Wed Mar 23 19:37:42 2016 Update recipes/recipe_modules in build/ for indexing output placeholders by names. The CL to index output placeholders by name is https://codereview.chromium.org/1773273003/. BUG= chromium:593198 Review URL: https://codereview.chromium.org/1820073003 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/build@299447 0039d316-1c4b-4281-b951-d872f2087c98 [modify] https://crrev.com/c155ba71c085f6f299de1e00f6b959d1f9b31819/infra/config/recipes.cfg [modify] https://crrev.com/c155ba71c085f6f299de1e00f6b959d1f9b31819/scripts/slave/recipe_modules/chromium/api.py [modify] https://crrev.com/c155ba71c085f6f299de1e00f6b959d1f9b31819/scripts/slave/recipe_modules/legion/__init__.py [modify] https://crrev.com/c155ba71c085f6f299de1e00f6b959d1f9b31819/scripts/slave/recipe_modules/legion/api.py [modify] https://crrev.com/c155ba71c085f6f299de1e00f6b959d1f9b31819/scripts/slave/recipe_modules/legion/example.expected/basic.json [modify] https://crrev.com/c155ba71c085f6f299de1e00f6b959d1f9b31819/scripts/slave/recipes/legion/legion.expected/basic.json
,
Mar 23 2016
The following revision refers to this bug: http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=85665 ------------------------------------------------------------------ r85665 | stgao@google.com | 2016-03-23T19:55:25.850554Z -----------------------------------------------------------------
,
Mar 23 2016
|
|||
►
Sign in to add a comment |
|||
Comment 1 by martiniss@chromium.org
, Mar 9 2016Is there a reason that you wouldn't just put both of the json outputs into the same json? E.g. with open(sys.argv[1], 'w') as f: f.write({ 'first': json.dumps([1, 2, 3]), 'second': json.dumps(["x", "y"]), }) ? I might just be missing the larger use case for this.