Pass args in file from task_runner to run_isolated

appengine/swarming/swarming_bot/bot_code/task_runner.py

 # Copyright 2013 The LUCI Authors. All rights reserved.
 # Use of this source code is governed under the Apache License, Version 2.0
 # that can be found in the LICENSE file.
 
 """Runs a Swarming task.
 
 Downloads all the necessary files to run the task, executes the command and
 streams results back to the Swarming server.
 
 The process exit code is 0 when the task was executed, even if the task itself
@@ skipping 65 matching lines @@
 
 
 def get_run_isolated():
   """Returns the path to itself to run run_isolated.
 
   Mocked in test to point to the real run_isolated.py script.
   """
   return [sys.executable, THIS_FILE, 'run_isolated']
 
 
-def get_isolated_cmd(
+def get_isolated_args(
     work_dir, task_details, isolated_result, bot_file, min_free_space):
   """Returns the command to call run_isolated. Mocked in tests."""
   assert (bool(task_details.command) !=
           bool(task_details.isolated and task_details.isolated.get('input')))
   bot_dir = os.path.dirname(work_dir)
   if os.path.isfile(isolated_result):
     os.remove(isolated_result)
-  cmd = get_run_isolated()
+  cmd = []
 
   if task_details.isolated:
     cmd.extend(
         [
           '-I', task_details.isolated['server'].encode('utf-8'),
           '--namespace', task_details.isolated['namespace'].encode('utf-8'),
         ])
     isolated_input = task_details.isolated.get('input')
     if isolated_input:
       cmd.extend(
@@ skipping 231 matching lines @@
   try:
     proc.wait(grace_period)
   except subprocess42.TimeoutError:
     logging.warning('SIGKILL finally due to %s', reason)
     proc.kill()
   exit_code = proc.wait()
   logging.info('Waiting for process exit in finally - done')
   return exit_code
 
 
+def fail_without_command(remote, bot_id, task_id, params, cost_usd_hour,
+                         task_start, exit_code, stdout):
+  now = monotonic_time()
+  params['cost_usd'] = cost_usd_hour * (now - task_start) / 60. / 60.
+  params['duration'] = now - task_start
+  params['io_timeout'] = False
+  params['hard_timeout'] = False
+  # Ignore server reply to stop.
+  remote.post_task_update(task_id, bot_id, params, (stdout, 0), 1)
+  return {
+    u'exit_code': exit_code,
+    u'hard_timeout': False,
+    u'io_timeout': False,
+    u'must_signal_internal_failure': None,
+    u'version': OUT_VERSION,
+  }
+
+
 def run_command(remote, task_details, work_dir, cost_usd_hour,
                 task_start, min_free_space, bot_file, extra_env):
   """Runs a command and sends packets to the server to stream results back.
 
   Implements both I/O and hard timeouts. Sends the packets numbered, so the
   server can ensure they are processed in order.
 
   Returns:
     Metadata dict with the execution result.
 
@@ skipping 14 matching lines @@
     # Don't even bother, the task was already canceled.
     return {
       u'exit_code': -1,
       u'hard_timeout': False,
       u'io_timeout': False,
       u'must_signal_internal_failure': None,
       u'version': OUT_VERSION,
     }
 
   isolated_result = os.path.join(work_dir, 'isolated_result.json')
-  cmd = get_isolated_cmd(
+  args_path = os.path.join(work_dir, 'run_isolated_args.json')
+  cmd = get_run_isolated()
+  cmd.extend(['-f', args_path])
+  args = get_isolated_args(
       work_dir, task_details, isolated_result, bot_file, min_free_space)
   # Hard timeout enforcement is deferred to run_isolated. Grace is doubled to
   # give one 'grace_period' slot to the child process and one slot to upload
   # the results back.
   task_details.hard_timeout = 0
   if task_details.grace_period:
     task_details.grace_period *= 2
 
   try:
     # TODO(maruel): Support both channels independently and display stderr in
     # red.
     env = os.environ.copy()
     for env_to_add in (task_details.env, extra_env):
       for key, value in (env_to_add or {}).iteritems():
         if not value:
           env.pop(key, None)
         else:
           env[key] = value
     logging.info('cmd=%s', cmd)
     logging.info('cwd=%s', work_dir)
     logging.info('env=%s', env)
+    fail_on_start = lambda exit_code, stdout: fail_without_command(
+        remote, bot_id, task_id, params, cost_usd_hour, task_start,
+        exit_code, stdout)
+
+    # We write args to a file since there may be more of them than the OS
+    # can handle.
+    try:
+      args_file = open(args_path, "w")

[M-A Ruel, 2016/10/24 16:26:52]

with open(...

[aludwin, 2016/10/24 17:05:48]

Do I put that inside a try-catch?

[M-A Ruel, 2016/10/24 17:11:53]

yes, so it looks like:

try:
  with open(args_path, 'w') as f:
    json.dump(args, f)
except (IOError, OSError) as e:
  ...


Notes:
- single quotes
- use f instead of args_file
- order the exception class name alphabetically

+      json.dump(args, args_file)
+      args_file.close()
+    except (OSError, IOError) as e:
+      return fail_on_start(
+          -1,
+          'Could not write args to %s: %s' % (args_path, e))
+
+    # Start the command
     try:
       assert cmd and all(isinstance(a, basestring) for a in cmd)
       proc = subprocess42.Popen(
           cmd,
           env=env,
           cwd=work_dir,
           detached=True,
           stdout=subprocess42.PIPE,
           stderr=subprocess42.STDOUT,
           stdin=subprocess42.PIPE)
     except OSError as e:
-      stdout = 'Command "%s" failed to start.\nError: %s' % (' '.join(cmd), e)
-      now = monotonic_time()
-      params['cost_usd'] = cost_usd_hour * (now - task_start) / 60. / 60.
-      params['duration'] = now - start
-      params['io_timeout'] = False
-      params['hard_timeout'] = False
-      # Ignore server reply to stop.
-      remote.post_task_update(task_id, bot_id, params, (stdout, 0), 1)
-      return {
-        u'exit_code': 1,
-        u'hard_timeout': False,
-        u'io_timeout': False,
-        u'must_signal_internal_failure': None,
-        u'version': OUT_VERSION,
-      }
+      return fail_on_start(
+          1,
+          'Command "%s" failed to start.\nError: %s' % (' '.join(cmd), e))
 
+    # Monitor the task
     output_chunk_start = 0
     stdout = ''
     exit_code = None
     had_io_timeout = False
     must_signal_internal_failure = None
     kill_sent = False
     timed_out = None
     try:
       calc = lambda: calc_yield_wait(
           task_details, start, last_io, timed_out, stdout)
@@ skipping 186 matching lines @@
     options.start = now
 
   try:
     load_and_run(
         options.in_file, options.swarming_server, options.cost_usd_hour,
         options.start, options.out_file, options.min_free_space,
         options.bot_file, options.auth_params_file)
     return 0
   finally:
     logging.info('quitting')