[GitHub] [airflow] mdediana commented on a change in pull request #9125: Add tests for Elasticsearch logs

[GitBox <gi...@apache.org>]

mdediana commented on a change in pull request #9125:
URL: https://github.com/apache/airflow/pull/9125#discussion_r434823137



##########
File path: tests/www/test_views.py
##########
@@ -2801,3 +2801,56 @@ def test_action_logging_post(self):
         self.session.commit()
         self.check_last_log("example_bash_operator", event="clear",
                             execution_date=self.EXAMPLE_DAG_DEFAULT_DATE)
+
+
+class TestRemoteLogs(TestBase):
+    DEFAULT_DATE = datetime(2017, 9, 1)
+    PARAMS = {
+        'dag_id': 'example_bash_operator',
+        'task_id': 'runme_0',
+        'execution_date': quote_plus(DEFAULT_DATE.isoformat()),
+        'try_number': 1,
+    }
+    GRAPH_ENDPOINT = '/graph?dag_id={dag_id}&root={task_id}'.format(**PARAMS)
+    LOG_ID_TEMPLATE = '{dag_id}-{task_id}-{execution_date}-{try_number}'
+    LOG_ID = LOG_ID_TEMPLATE.format(**PARAMS)
+
+    def test_elasticsearch_not_configured(self):
+        with self.capture_templates() as templates:
+            self.client.get(self.GRAPH_ENDPOINT, follow_redirects=True)
+
+            self.assertFalse(templates[0].local_context['show_external_logs'])
+
+    @parameterized.expand([
+        # Common case
+        ('localhost:5601/{log_id}', LOG_ID_TEMPLATE, 'https://localhost:5601/' + LOG_ID),
+        # Ignore template if "{log_id}"" is missing in the URL
+        ('localhost:5601', LOG_ID_TEMPLATE, 'https://localhost:5601'),
+        # Automatically add https://
+        ('http://localhost:5601', LOG_ID_TEMPLATE, 'https://http://localhost:5601'),

Review comment:
       Creating an invalid URL may be an unintended consequence of the automatic scheme addition (even though users are advised not to do so as a comment in the config file). Also, it is impossible to run a frontend accessible via http (a local setup, for example). Maybe changing this behavior is a good idea: use the scheme defined by the user, if missing default to `https`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org