swarming: change meaning of inputs_ref

appengine/swarming/server/task_request.py

Index: appengine/swarming/server/task_request.py
diff --git a/appengine/swarming/server/task_request.py b/appengine/swarming/server/task_request.py
index 3182acca3b6d7d0bd4d9358eda3f0233ece01ca8..c8eba11a8fdd9d682b3179ad1cb904d927d8a695 100644
--- a/appengine/swarming/server/task_request.py
+++ b/appengine/swarming/server/task_request.py
@@ -238,6 +238,21 @@ class FilesRef(ndb.Model):
           'isolated requires server and namespace')
 
 
+class IsolatedOutputsTarget(ndb.Model):
+  """Defines a target of isolated output."""
+  # The hostname of the isolated server to use.
+  isolatedserver = ndb.StringProperty(
+      validator=_validate_hostname, indexed=False)
+  # Namespace on the isolate server.
+  namespace = ndb.StringProperty(validator=_validate_namespace, indexed=False)
+
+  def _pre_put_hook(self):
+    super(IsolatedOutputsTarget, self)._pre_put_hook()
+    if not self.isolatedserver or not self.namespace:
+      raise datastore_errors.BadValueError(
+        'isolated outputs target requires server and namespace')
+
+
 class CipdPackage(ndb.Model):
   """A CIPD package to install in $CIPD_PATH and $PATH before task execution.
 
@@ -284,6 +299,10 @@ class TaskProperties(ndb.Model):
   # File inputs of the task. Only inputs_ref or command&data can be specified.
   inputs_ref = ndb.LocalStructuredProperty(FilesRef)
 
+  # Isolate output settings. Mutually exclusive with inputs_ref.
+  # If inputs_ref is not None, its server and namespace must be used for output.
+  outputs_target = ndb.LocalStructuredProperty(IsolatedOutputsTarget)

[M-A Ruel, 2016/05/03 14:01:23]

Argh, in retrospect maybe I should have used instead:

input_isolated = String
isolated_server = <server, metadata e.g. namespacE>
output_isolated = String

Do you think it's worth migrating over time the entities to this model?

[nodir, 2016/05/03 16:29:30]

No

[M-A Ruel, 2016/05/05 11:52:10]

Ok I thought about this a lot. I think it's a bad idea to add this. I think we
should converge to what I proposed above but this can be done in two steps so
you don't have to get bogged down with this.

So I'd recommend to unconditionally set inputs_ref.isolatedserver and namespace.
This requires updating the client code a bit but that's much simpler overall and
more future proof, as we can then refactor as I listed above without having to
have 3 different possible configurations.

At the top of this class I'd add a comment:

TODO(maruel):
- Convert inputs_ref and _TaskResultCommon.outputs_ref as:
  - input = String which is the isolated input, if any
  - isolated_server = <server, metadata e.g. namespace> which is a simplified
version of FilesRef
  - _TaskResultCommon.output = String which is isolated output, if any.

Don't worry I'm not asking you to do that, just add the TODO. 

[nodir, 2016/05/10 18:17:39]

Done, updated CL description.

+
   # A list of CIPD packages to install $CIPD_PATH and $PATH before task
   # execution.
   packages = ndb.LocalStructuredProperty(CipdPackage, repeated=True)
@@ -369,6 +388,11 @@ class TaskProperties(ndb.Model):
         raise datastore_errors.BadValueError('extra_args require inputs_ref')
       if self.inputs_ref:
         self.inputs_ref._pre_put_hook()
+        if self.outputs_target:
+          raise datastore_errors.BadValueError(
+              'do not specify outputs_target with inputs_ref')
+      elif self.outputs_target:
+        self.outputs_target._pre_put_hook()
 
       package_names = set()
       for p in self.packages:
@@ -387,7 +411,6 @@ class TaskProperties(ndb.Model):
             'use instance IDs or tags as package versions')
 
 
-
 class TaskRequest(ndb.Model):
   """Contains a user request.