Scottx611x/provide vis tools with url to their input data (#2677)

* Bump django_docker_engine req. * Remove temp assignment to `client` * Update DockerClientWrapper init; No slot for data_dir arg now * Provide detail route: `container_input_data` to fetch a Tool's input data as JSON * Add test coverage for container input data detail route * Use already available instance of DockerClientWrapper * Make _django_docker_client public * Launch VisualizationTools asynchronously; The AutoRelaunchProxy now provides a nice "Please Wait" page, so this seems like a natural move * Update tests that depended on VisualizationTool.launch being synchronous * Fix out of date django_docker_engine specifics in tests * Fix test after method was renamed * Add reference to container_input_data detail view * Use `None` for now to satisfy django_docker_engine arg; This should be removed on django_docker_engine side * Return 404s where appropriate in ToolsViewSet * Update tests to assert the newly added 404s * Fix typo * Fix docstring * Fit this on one line * Fit this on one line as well * Add comment about running containers asynchronously * Remove code related to `ToolDefinition.container_input_path` now that we provide this info as a url * Bump django_docker_engine req * Remove need for temporary arg
refinery-platform · Apr 13, 2018 · 1fbaf2c · 1fbaf2c
1 parent 99658a7
commit 1fbaf2c
Show file tree

Hide file tree

Showing 18 changed files with 175 additions and 131 deletions.
diff --git a/refinery/tool_manager/migrations/0026_remove_tooldefinition_container_input_path.py b/refinery/tool_manager/migrations/0026_remove_tooldefinition_container_input_path.py
@@ -0,0 +1,18 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('tool_manager', '0025_auto_20180226_2153'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='tooldefinition',
+            name='container_input_path',
+        ),
+    ]
diff --git a/refinery/tool_manager/models.py b/refinery/tool_manager/models.py
@@ -2,6 +2,7 @@
 import json
 import logging
 import re
+from urlparse import urljoin
 import uuid
 
 from django.conf import settings
@@ -11,29 +12,37 @@
 from django.http import JsonResponse
 
 import bioblend
-from bioblend.galaxy.dataset_collections import (CollectionDescription,
-                                                 CollectionElement,
-                                                 HistoryDatasetElement)
+from bioblend.galaxy.dataset_collections import (
+    CollectionDescription, CollectionElement, HistoryDatasetElement
+)
 from constants import UUID_RE
-from django_docker_engine.container_managers.docker_engine import \
+from django_docker_engine.container_managers.docker_engine import (
     ExpectedPortMissing
-from django_docker_engine.docker_utils import (DockerClientWrapper,
-                                               DockerContainerSpec)
+)
+from django_docker_engine.docker_utils import (
+    DockerClientRunWrapper, DockerClientSpec, DockerContainerSpec
+)
 from django_extensions.db.fields import UUIDField
 from docker.errors import APIError, NotFound
 
 from analysis_manager.models import AnalysisStatus
-from analysis_manager.tasks import (_galaxy_file_import, get_taskset_result,
-                                    run_analysis)
+from analysis_manager.tasks import (
+    _galaxy_file_import, get_taskset_result, run_analysis
+)
 from analysis_manager.utils import create_analysis, validate_analysis_config
-from core.models import (INPUT_CONNECTION, OUTPUT_CONNECTION, Analysis,
-                         AnalysisNodeConnection, DataSet, OwnableResource,
-                         Workflow)
+from core.models import (
+    INPUT_CONNECTION, OUTPUT_CONNECTION, Analysis, AnalysisNodeConnection,
+    DataSet, OwnableResource, Workflow
+)
+from core.utils import get_absolute_url
 from data_set_manager.models import Node
-from data_set_manager.utils import (get_file_url_from_node_uuid,
-                                    get_solr_response_json)
+from data_set_manager.utils import (
+    get_file_url_from_node_uuid, get_solr_response_json
+)
 from file_store.models import FileType
 
+from .tasks import start_container
+
 logger = logging.getLogger(__name__)
 
 
@@ -180,10 +189,6 @@ class ToolDefinition(models.Model):
     file_relationship = models.ForeignKey(FileRelationship)
     parameters = models.ManyToManyField(Parameter)
     image_name = models.CharField(max_length=255, blank=True)
-    container_input_path = models.CharField(
-        max_length=500,
-        blank=True
-    )
     annotation = models.TextField()
     workflow = models.ForeignKey(Workflow, null=True)
 
@@ -285,6 +290,7 @@ class Tool(OwnableResource):
     REFINERY_FILE_UUID = "refinery_file_uuid"
     TOOL_LAUNCH_CONFIGURATION = "tool_launch_configuration"
     TOOL_URL = "tool_url"
+    TOOL_API_ROOT = "/api/v2/tools/"
 
     dataset = models.ForeignKey(DataSet)
     analysis = models.OneToOneField(Analysis, blank=True, null=True)
@@ -306,12 +312,28 @@ def __str__(self):
         return "Tool: {}".format(self.get_tool_name())
 
     @property
-    def _django_docker_client(self):
-        return DockerClientWrapper(settings.DJANGO_DOCKER_ENGINE_DATA_DIR)
+    def django_docker_client(self):
+        return DockerClientRunWrapper(
+            DockerClientSpec(
+                settings.DJANGO_DOCKER_ENGINE_DATA_DIR,
+                input_json_url=get_absolute_url(self.container_input_json_url)
+            )
+        )
 
     @property
     def relaunch_url(self):
-        return "/api/v2/tools/{}/relaunch/".format(self.uuid)
+        return urljoin(self.TOOL_API_ROOT, "{}/relaunch/".format(self.uuid))
+
+    @property
+    def container_input_json_url(self):
+        """
+        Return the url that will expose a Tool's input data (as JSON) on
+        GET requests
+        """
+        return urljoin(
+            self.TOOL_API_ROOT,
+            "{}/container_input_data/".format(self.uuid)
+        )
 
     def _get_owner_info_as_dict(self):
         user = self.get_owner()
@@ -423,7 +445,7 @@ def is_running(self):
             return self.analysis.running()
 
         try:
-            self._django_docker_client.lookup_container_url(
+            self.django_docker_client.lookup_container_url(
                 self.container_name
             )
             return True
@@ -467,9 +489,9 @@ class Meta:
             ('read_%s' % verbose_name, 'Can read %s' % verbose_name),
         )
 
-    def _create_container_input_dict(self):
+    def get_container_input_dict(self):
         """
-        Creat a dictionary containing information that Dockerized
+        Create a dictionary containing information that Dockerized
         Visualizations will have access to
         """
         return {
@@ -488,9 +510,17 @@ def _create_container_input_dict(self):
                 self.tool_definition.get_extra_directories()
         }
 
+    def create_container_spec(self):
+        return DockerContainerSpec(
+            image_name=self.tool_definition.image_name,
+            container_name=self.container_name,
+            labels={self.uuid: ToolDefinition.VISUALIZATION},
+            extra_directories=self.tool_definition.get_extra_directories()
+        )
+
     def _check_max_running_containers(self):
         max_containers = settings.DJANGO_DOCKER_ENGINE_MAX_CONTAINERS
-        if len(self._django_docker_client.list()) >= max_containers:
+        if len(self.django_docker_client.list()) >= max_containers:
             raise VisualizationToolError('Max containers')
 
     def _get_detailed_nodes_dict(self, node_uuid_list):
@@ -556,17 +586,9 @@ def launch(self):
         """
         self._check_max_running_containers()
 
-        container = DockerContainerSpec(
-            image_name=self.tool_definition.image_name,
-            container_name=self.container_name,
-            labels={self.uuid: ToolDefinition.VISUALIZATION},
-            container_input_path=self.tool_definition.container_input_path,
-            input=self._create_container_input_dict(),
-            extra_directories=self.tool_definition.get_extra_directories()
-        )
-
-        self._django_docker_client.run(container)
-
+        # Pulls docker image if it doesn't exist yet, and launches container
+        # asynchronously
+        start_container.delay(self)
         return JsonResponse({Tool.TOOL_URL: self.get_relative_container_url()})
 
 
@@ -1426,9 +1448,7 @@ def remove_tool_container(sender, instance, *args, **kwargs):
     VisualizationTool's launch.
     """
     try:
-        DockerClientWrapper(
-            settings.DJANGO_DOCKER_ENGINE_DATA_DIR
-        ).purge_by_label(instance.uuid)
+        instance.django_docker_client.purge_by_label(instance.uuid)
     except APIError as e:
         logger.error("Couldn't purge container for Tool with UUID: %s %s",
                      instance.uuid, e)
diff --git a/refinery/tool_manager/schemas/ToolDefinition.json b/refinery/tool_manager/schemas/ToolDefinition.json
@@ -37,7 +37,6 @@
         "file_relationship",
         "parameters",
         "extra_directories",
-        "container_input_path",
         "image_name"
       ],
       "properties": {
@@ -60,9 +59,6 @@
           "type": "string",
           "enum": ["VISUALIZATION"]
         },
-        "container_input_path": {
-          "type": "string"
-        },
         "extra_directories": {
           "type": "array",
           "items": {

diff --git a/refinery/tool_manager/tasks.py b/refinery/tool_manager/tasks.py
@@ -7,5 +7,13 @@
 @task()
 def django_docker_cleanup():
     # TODO: Specify manager, if not default
-    client = DockerClientWrapper(settings.DJANGO_DOCKER_ENGINE_DATA_DIR)
-    client.purge_inactive(settings.DJANGO_DOCKER_ENGINE_SECONDS_INACTIVE)
+    DockerClientWrapper().purge_inactive(
+        settings.DJANGO_DOCKER_ENGINE_SECONDS_INACTIVE
+    )
+
+
+@task()
+def start_container(visualization_tool):
+    visualization_tool.django_docker_client.run(
+        visualization_tool.create_container_spec()
+    )
diff --git a/..._manager/test_data/visualizations/LIST_visualization_bad_extra_directories_structure.json b/..._manager/test_data/visualizations/LIST_visualization_bad_extra_directories_structure.json
@@ -2,7 +2,6 @@
   "name": "Test LIST Visualization bad extra_directories",
   "tool_type": "VISUALIZATION",
   "annotation": {
-    "container_input_path": "/var/www/html/input.json",
     "extra_directories": {"/test": "dir"},
     "image_name": "nginx:1.10.3-alpine",
     "description": "This visualization does really cool things",

diff --git a/...nery/tool_manager/test_data/visualizations/LIST_visualization_good_extra_directories.json b/...nery/tool_manager/test_data/visualizations/LIST_visualization_good_extra_directories.json
@@ -2,7 +2,6 @@
   "name": "Test LIST Visualization bad extra_directories",
   "tool_type": "VISUALIZATION",
   "annotation": {
-    "container_input_path": "/var/www/html/input.json",
     "extra_directories": [],
     "image_name": "nginx:1.10.3-alpine",
     "description": "This visualization does really cool things",

diff --git a/...y/tool_manager/test_data/visualizations/LIST_visualization_missing_extra_directories.json b/...y/tool_manager/test_data/visualizations/LIST_visualization_missing_extra_directories.json
@@ -2,7 +2,6 @@
   "name": "Test LIST Visualization bad extra_directories",
   "tool_type": "VISUALIZATION",
   "annotation": {
-    "container_input_path": "/var/www/html/input.json",
     "image_name": "nginx:1.10.3-alpine",
     "description": "This visualization does really cool things",
     "parameters": [],

diff --git a/refinery/tool_manager/test_data/visualizations/bad_filetype.json b/refinery/tool_manager/test_data/visualizations/bad_filetype.json
@@ -2,7 +2,6 @@
   "name": "IGV with a bad filetype specified",
   "tool_type": "VISUALIZATION",
   "annotation": {
-    "container_input_path": "/var/www/data/input.json",
     "extra_directories": ["/refinery-data"],
     "image_name": "gehlenborglab/docker_igv_js:v0.0.3",
     "description": "This visualization does really cool things... with IGV",

diff --git a/refinery/tool_manager/test_data/visualizations/hello_world.json b/refinery/tool_manager/test_data/visualizations/hello_world.json
@@ -2,7 +2,6 @@
   "name": "Test LIST Visualization Hello World",
   "tool_type": "VISUALIZATION",
   "annotation": {
-    "container_input_path": "/var/www/html/input.json",
     "extra_directories": ["/refinery-data"],
     "image_name": "nginx:1.10.3-alpine",
     "description": "This visualization does really cool things",

diff --git a/refinery/tool_manager/test_data/visualizations/higlass.json b/refinery/tool_manager/test_data/visualizations/higlass.json
@@ -2,7 +2,6 @@
   "name": "Test LIST Visualization HiGlass",
   "tool_type": "VISUALIZATION",
   "annotation": {
-    "container_input_path": "/data/input.json",
     "extra_directories": ["/refinery-data"],
     "image_name": "scottx611x/refinery-higlass-docker:v0.1.0",
     "description": "This visualization does really cool things... with Higlass",

diff --git a/refinery/tool_manager/test_data/visualizations/igv.json b/refinery/tool_manager/test_data/visualizations/igv.json
@@ -2,7 +2,6 @@
   "name": "Test LIST Visualization IGV",
   "tool_type": "VISUALIZATION",
   "annotation": {
-    "container_input_path": "/var/www/data/input.json",
     "extra_directories": ["/refinery-data"],
     "image_name": "gehlenborglab/docker_igv_js:v0.0.3",
     "description": "This visualization does really cool things... with IGV",

diff --git a/refinery/tool_manager/test_data/visualizations/no_docker_image_version.json b/refinery/tool_manager/test_data/visualizations/no_docker_image_version.json
@@ -2,7 +2,6 @@
   "name": "IGV with no docker image version specified",
   "tool_type": "VISUALIZATION",
   "annotation": {
-    "container_input_path": "/var/www/data/input.json",
     "extra_directories": ["/refinery-data"],
     "image_name": "gehlenborglab/docker_igv_js",
     "description": "This visualization does really cool things... with IGV",