New issue
Advanced search Search tips

Issue 728874 link

Starred by 1 user
Status: Fixed
Owner:
mar...@chromium.org
Closed: Nov 2017
Cc:
mar...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Task environment variable must be applied only on target process

Project Member Reported by mar...@chromium.org, Jun 2 2017 
Right now _run_manifest() in
https://github.com/luci/luci-py/blob/master/appengine/swarming/swarming_bot/bot_code/bot_main.py 
applies the task environment variable.

This is totally incorrect, then environment variables must be applied to the target process as started by run_isolated.py, two process deep (there's task_runner.py in between).

This means passing the environment dict down the two processes. This could be done via task_in_file as the environment variables could be long and could overflow the command line limit.
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 28 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/fc3c34e59202eeb0e575c8a0bf0c2cfad223620f

commit fc3c34e59202eeb0e575c8a0bf0c2cfad223620f
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Tue Nov 28 23:45:10 2017

swarming_bot: migrate env mutation from task_runner to run_isolated.

Update the smoke test to not only test setting environment variables, but also
environment suppression. This enables removing variables set by the bot like
SWARMING_TASK_ID.

R=iannucci@chromium.org
Bug:  728874 
Change-Id: I7c2679aae8d4a01371e07fc5dde7ff2d1e9e4338
Reviewed-on: https://chromium-review.googlesource.com/786123
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/fc3c34e59202eeb0e575c8a0bf0c2cfad223620f/appengine/swarming/local_smoke_test.py
[modify] https://crrev.com/fc3c34e59202eeb0e575c8a0bf0c2cfad223620f/appengine/swarming/swarming_bot/bot_code/task_runner.py
[modify] https://crrev.com/fc3c34e59202eeb0e575c8a0bf0c2cfad223620f/appengine/swarming/swarming_bot/bot_code/task_runner_test.py
[modify] https://crrev.com/fc3c34e59202eeb0e575c8a0bf0c2cfad223620f/client/run_isolated.py
[modify] https://crrev.com/fc3c34e59202eeb0e575c8a0bf0c2cfad223620f/client/tests/run_isolated_test.py

Comment 2 by mar...@chromium.org, Nov 29 2017

Labels: -Restrict-View-Google
Owner: mar...@chromium.org
Status: Fixed (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 29 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/f61b1c62d72cb6229df05266d2188bfac753ed97

commit f61b1c62d72cb6229df05266d2188bfac753ed97
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Wed Nov 29 18:48:23 2017

Fix regression in fc3c34e59202ee and bfab799cddab1

subprocess.Popen() throws on Windows if env is not purely str instances. When
passing arguments via a json file instead of normal arguments, the argument
values are unicode instances instead of str.

R=iannucci@chromium.org
Bug:  728874 , 786503
Change-Id: I3cb5c751557285eb487b7b5d2029ec222e602032
Reviewed-on: https://chromium-review.googlesource.com/796935
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/f61b1c62d72cb6229df05266d2188bfac753ed97/client/run_isolated.py
►

Sign in to add a comment