From 5bcb10b7213671bda25accc6484b2509c23b0133 Mon Sep 17 00:00:00 2001
From: Jorrit Schaap <schaap@astron.nl>
Date: Mon, 9 Nov 2020 12:37:37 +0100
Subject: [PATCH] TMSS-322: reimplemented this feature. As stated in the
 ticket: such a convenience log_url is not part of the Subtask model, but can
 be a view on the model. So, implemeted it in the viewset. Now, fetching
 subtasks from the TMSS db is superfast again.

---
 SAS/TMSS/client/lib/tmss_http_rest_client.py  |  7 ++-
 SAS/TMSS/src/tmss/settings.py                 |  6 ---
 .../src/tmss/tmssapp/models/scheduling.py     | 25 ----------
 .../tmss/tmssapp/serializers/scheduling.py    |  2 +-
 .../src/tmss/tmssapp/viewsets/scheduling.py   | 46 ++++++++++++++++++-
 SAS/TMSS/test/t_subtasks.py                   | 46 ++++++++++---------
 6 files changed, 76 insertions(+), 56 deletions(-)

diff --git a/SAS/TMSS/client/lib/tmss_http_rest_client.py b/SAS/TMSS/client/lib/tmss_http_rest_client.py
index 0fb13c71c65..48df33a1cab 100644
--- a/SAS/TMSS/client/lib/tmss_http_rest_client.py
+++ b/SAS/TMSS/client/lib/tmss_http_rest_client.py
@@ -156,10 +156,13 @@ class TMSSsession(object):
 
         return self.get_path_as_json_object("subtask", clauses)
 
+    def get_full_url_for_path(self, path: str) -> str:
+        '''get the full URL for the given path'''
+        return '%s/%s' % (self.base_url, path.strip('/'))
+
     def get_path_as_json_object(self, path: str, params={}) -> object:
         '''get resource at the given path, interpret it as json, and return it as as native object (usually a dict or a list of dicts)'''
-        full_url = '%s/%s' % (self.base_url, path.strip('/'))
-        return self.get_url_as_json_object(full_url, params=params)
+        return self.get_url_as_json_object(self.get_full_url_for_path(path=path), params=params)
 
     def get_url_as_json_object(self, full_url: str, params={}) -> object:
         '''get resource at the given full url (including http://<base_url>, interpret it as json, and return it as as native object (usually a dict or a list of dicts)'''
diff --git a/SAS/TMSS/src/tmss/settings.py b/SAS/TMSS/src/tmss/settings.py
index 909caac96ed..eecff2fb7eb 100644
--- a/SAS/TMSS/src/tmss/settings.py
+++ b/SAS/TMSS/src/tmss/settings.py
@@ -355,9 +355,3 @@ SWAGGER_SETTINGS = {
     },
 
 }
-
-# TODO Do I need distinguish more between Test and Production Environment??
-# maybe a local file in Development environment for test purposes
-SCU = "http://scu199" if isDevelopmentEnvironment() or isTestEnvironment() else "http://scu001"
-PIPELINE_SUBTASK_LOG_URL = SCU + ".control.lofar:7412/tasks/%s/log.html"
-OBSERVATION_SUBTASK_LOG_URL = "https://proxy.lofar.eu/inspect/%s/rtcp-%s.errors"
diff --git a/SAS/TMSS/src/tmss/tmssapp/models/scheduling.py b/SAS/TMSS/src/tmss/tmssapp/models/scheduling.py
index a6257840944..1ccb3a4a027 100644
--- a/SAS/TMSS/src/tmss/tmssapp/models/scheduling.py
+++ b/SAS/TMSS/src/tmss/tmssapp/models/scheduling.py
@@ -250,31 +250,6 @@ class Subtask(BasicCommon):
             # update the previous state value
             self.__original_state_id = self.state_id
 
-    @property
-    def log_url(self):
-        """
-        Return the link to the pipeline log in case of pipeline or
-        link to COBALT error log in case of an observation
-        otherwise just an empty string
-        """
-        if self.specifications_template.type.value == SubtaskType.Choices.OBSERVATION.value:
-            url = settings.OBSERVATION_SUBTASK_LOG_URL % (self.id, self.id)
-        elif self.specifications_template.type.value == SubtaskType.Choices.PIPELINE.value:
-            # Get RADBID, subtask must be at least 'scheduled' to exist in radb
-            # If RA is not started don't wait longer than 10 seconds
-            with RADBRPC.create(timeout=10) as radbrpc:
-                try:
-                    radb_id = radbrpc.getTask(tmss_id=self.id)
-                except:
-                    radb_id = None
-            if radb_id is None:
-               url = "not available (missing radbid)"
-            else:
-               url = settings.PIPELINE_SUBTASK_LOG_URL % radb_id['id']
-        else:
-            url = ""
-        return url
-
 
 class SubtaskStateLog(BasicCommon):
     """
diff --git a/SAS/TMSS/src/tmss/tmssapp/serializers/scheduling.py b/SAS/TMSS/src/tmss/tmssapp/serializers/scheduling.py
index a2a10449dae..85d7bd21c54 100644
--- a/SAS/TMSS/src/tmss/tmssapp/serializers/scheduling.py
+++ b/SAS/TMSS/src/tmss/tmssapp/serializers/scheduling.py
@@ -85,7 +85,7 @@ class SubtaskSerializer(RelationalHyperlinkedModelSerializer):
     class Meta:
         model = models.Subtask
         fields = '__all__'
-        extra_fields = ['cluster_value', 'log_url']
+        extra_fields = ['cluster_value']
 
 
 class SubtaskInputSerializer(RelationalHyperlinkedModelSerializer):
diff --git a/SAS/TMSS/src/tmss/tmssapp/viewsets/scheduling.py b/SAS/TMSS/src/tmss/tmssapp/viewsets/scheduling.py
index 54ed8f42527..2bc7b1814e5 100644
--- a/SAS/TMSS/src/tmss/tmssapp/viewsets/scheduling.py
+++ b/SAS/TMSS/src/tmss/tmssapp/viewsets/scheduling.py
@@ -16,9 +16,10 @@ from drf_yasg.utils import swagger_auto_schema
 from drf_yasg.inspectors import SwaggerAutoSchema
 
 from rest_framework.decorators import action
-from django.http import HttpResponse, JsonResponse
+from django.http import HttpResponse, JsonResponse, HttpResponseRedirect, HttpResponseNotFound
 from rest_framework.response import Response as RestResponse
 
+from lofar.common import isProductionEnvironment, isTestEnvironment
 from lofar.sas.tmss.tmss.tmssapp.viewsets.lofar_viewset import LOFARViewSet
 from lofar.sas.tmss.tmss.tmssapp import models
 from lofar.sas.tmss.tmss.tmssapp import serializers
@@ -209,6 +210,49 @@ class SubtaskViewSet(LOFARViewSet):
         return RestResponse(serializer.data)
 
 
+    @swagger_auto_schema(responses={302: 'A redirect url to the task log for this Subtask.',
+                                    403: 'forbidden'},
+                         operation_description="Get the task log for this Subtask.")
+    @action(methods=['get'], detail=True)
+    def task_log(self, request, pk=None):
+        """
+        Return a redirect to the the link to the pipeline log in case of pipeline or
+        link to COBALT error log in case of an observation.
+        """
+        subtask = get_object_or_404(models.Subtask, pk=pk)
+
+        # redirect to cobalt log served at proxy.lofar.eu
+        if subtask.specifications_template.type.value == models.SubtaskType.Choices.OBSERVATION.value:
+            url = "https://proxy.lofar.eu/inspect/%s/rtcp-%s.errors" % (subtask.id, subtask.id)
+            return HttpResponseRedirect(redirect_to=url)
+
+        # redirect to pipeline log served via webscheduler
+        if subtask.specifications_template.type.value == models.SubtaskType.Choices.PIPELINE.value:
+            # import here and not at top of module to "loosen" dependency on external packages, such as in this case the RADB RPC.
+            from lofar.sas.resourceassignment.resourceassignmentservice.rpc import RADBRPC
+            # Get RADBID, subtask must be at least 'scheduled' to exist in radb
+            try:
+                with RADBRPC.create(timeout=2) as radbrpc:
+                    radb_id = radbrpc.getTask(tmss_id=subtask.id)
+
+                    if radb_id is None:
+                        return HttpResponseNotFound(
+                            content='No RADB task found for subtask id=%s type="%s status=%s". Cannot redirect to pipeline log.' % (
+                                subtask.id, subtask.specifications_template.type.value, subtask.state))
+
+                    WEBSCHEDULER_URL = "http://scu001.control.lofar:7412" if isProductionEnvironment() else \
+                                       "http://scu199.control.lofar:7412" if isTestEnvironment() else \
+                                       "http://localhost:7412"
+
+                    url = "%s/tasks/%s/log.html" % (WEBSCHEDULER_URL, radb_id)
+                    return HttpResponseRedirect(redirect_to=url)
+            except Exception as e:
+                return HttpResponseNotFound(content='No RADB task found for subtask id=%s type="%s". Cannot redirect to pipeline log.' % (subtask.id, subtask.specifications_template.type.value))
+
+        # unknown log
+        return HttpResponseNotFound(content='No log (url) available for subtask id=%s type="%s"' % (subtask.id, subtask.specifications_template.type.value) )
+
+
     @swagger_auto_schema(responses={200: 'The input dataproducts of this subtask.',
                                     403: 'forbidden'},
                          operation_description="Get the input dataproducts of this subtask.")
diff --git a/SAS/TMSS/test/t_subtasks.py b/SAS/TMSS/test/t_subtasks.py
index 4f40e9c53b8..b9021a86f94 100755
--- a/SAS/TMSS/test/t_subtasks.py
+++ b/SAS/TMSS/test/t_subtasks.py
@@ -34,12 +34,7 @@ tmss_test_env.populate_schemas()
 
 
 from lofar.sas.tmss.test.tmss_test_data_django_models import *
-
-# import and setup rest test data creator
-from lofar.sas.tmss.test.tmss_test_data_rest import TMSSRESTTestDataCreator
-
 from lofar.sas.tmss.tmss.tmssapp import models
-
 from lofar.sas.tmss.tmss.tmssapp.subtasks import *
 
 
@@ -347,6 +342,31 @@ class SubtaskInputSelectionFilteringTest(unittest.TestCase):
         selection = {'sap': ['target0'], 'is_relevant': True}
         self.assertFalse(specifications_doc_meets_selection_doc(specs, selection))
 
+    def test_links_to_log_files(self):
+        """
+        Test redirect urls to subtask logfiles.
+        """
+
+        # the link to log files is a 'view' on the subtask, and NOT part of the subtask model.
+        # the link is served as an action on the REST API, redirecting to externally served log files.
+        # check/test the redirect urls.
+        with tmss_test_env.create_tmss_client() as client:
+            # observation
+            subtask_observation = create_subtask_object_for_testing("observation", "defined")
+            response = client.session.get(url=client.get_full_url_for_path('/subtask/%s/task_log' % (subtask_observation.id,)), allow_redirects=False)
+            self.assertTrue(response.is_redirect)
+            self.assertIn("proxy.lofar.eu", response.headers['Location'])
+            self.assertIn("rtcp-%s.errors" % subtask_observation.id, response.headers['Location'])
+
+            # pipeline
+            subtask_pipeline = create_subtask_object_for_testing("pipeline", "defined")
+            response = client.session.get(url=client.get_full_url_for_path('/subtask/%s/task_log' % (subtask_pipeline.id,)), allow_redirects=False)
+            self.assertEqual(404, response.status_code) # no log (yet) for unscheduled pipeline
+
+            # other (qa_plots)
+            subtask_qa_plots = create_subtask_object_for_testing("qa_plots", "defined")
+            self.assertEqual(404, response.status_code) # no log for other subtasktypes
+
 
 class SettingTest(unittest.TestCase):
 
@@ -359,22 +379,6 @@ class SettingTest(unittest.TestCase):
         with self.assertRaises(SubtaskSchedulingException):
             schedule_observation_subtask(obs_st)
 
-    def test_links_to_log_files(self):
-        """
-        Test if the links to logging of a subtasks is correct:
-        For an observation the subtaskid is in the logging url
-        For a pipeline the radbid of the subtaskid is in the link, BUT because RA is not started is should
-        return "not available"
-        All other subtask types (like qa) should have an empty string (no logging)
-        """
-        subtask_pipeline = create_subtask_object_for_testing("pipeline", "defined")
-        subtask_qa_plots = create_subtask_object_for_testing("qa_plots", "defined")
-        subtask_observation = create_subtask_object_for_testing("observation", "defined")
-
-        self.assertIn("proxy.lofar.eu", subtask_observation.log_url)
-        self.assertIn("rtcp-%s.errors" % subtask_observation.id, subtask_observation.log_url)
-        self.assertIn("not available", subtask_pipeline.log_url)
-        self.assertEqual("", subtask_qa_plots.log_url)
 
 
 if __name__ == "__main__":
-- 
GitLab