Return non-200 response if Airflow log request failed

This patch modifies API behavior for
GET /v1.0/actions/{action_id}/steps/{step_id}/logs
such way:
- it returns the same status code as Airflow HTTP request returned
  if Airflow responds with a status code of 400 or greater,
- it returns 500 error status code if an exception happens during
  Airflow HTTP request (200 was before).

Warning: this change breaks API backward compatibility, now a client
could get 4xx or 5xx codes proxied from Airflow.

Change-Id: Ic5dceb3abc34415d21b4d8d4e71b4e5661a7363d
This commit is contained in:
Andrey Volkov 2018-09-28 14:54:19 -07:00
parent 12d79ce423
commit 0e6486be8b
3 changed files with 49 additions and 8 deletions

View File

@ -1151,6 +1151,10 @@ try={int try_number}
Responses
'''''''''
200 OK
4xx or 5xx
A 4xx or 5xx code will be returned if some error happens during
Airflow HTTP request or Airflow responds with a status code of 400 or greater.
Example
'''''''

View File

@ -21,6 +21,7 @@ from oslo_config import cfg
from shipyard_airflow import policy
from shipyard_airflow.control.base import BaseResource
from shipyard_airflow.control.helpers.action_helper import ActionsHelper
from shipyard_airflow.errors import ApiError
CONF = cfg.CONF
LOG = logging.getLogger(__name__)
@ -116,18 +117,29 @@ class ActionsStepsLogsResource(BaseResource):
"""
Retrieve Logs
"""
try:
LOG.debug("Retrieving Airflow logs...")
LOG.debug("Retrieving Airflow logs...")
try:
response = requests.get(
log_endpoint,
timeout=(
CONF.requests_config.airflow_log_connect_timeout,
CONF.requests_config.airflow_log_read_timeout))
return response.text
except requests.exceptions.RequestException as e:
LOG.info(e)
LOG.info("Unable to retrieve requested logs")
return []
LOG.exception(e)
raise ApiError(
title='Log retrieval error',
description='Exception happened during Airflow API request',
status=falcon.HTTP_500)
if response.status_code >= 400:
LOG.info('Airflow endpoint returned error status code %s, '
'content %s. Response code will be bubbled up',
response.status_code, response.text)
raise ApiError(
title='Log retrieval error',
description='Airflow endpoint returned error status code',
status=getattr(
falcon,
'HTTP_%d' % response.status_code,
falcon.HTTP_500))
return response.text

View File

@ -15,8 +15,13 @@ from datetime import datetime
from unittest import mock
from unittest.mock import patch
import falcon
import pytest
import requests
from shipyard_airflow.control.action.actions_steps_id_logs_api import \
ActionsStepsLogsResource
from shipyard_airflow.errors import ApiError
from tests.unit.control import common
# Define Global Variables
@ -194,3 +199,23 @@ class TestActionsStepsLogsEndpoint():
result = action_logs_resource.retrieve_logs(log_endpoint)
assert result == XCOM_RUN_LOGS
@mock.patch('requests.get')
def test_retrieve_logs_404(self, mock_get):
mock_get.return_value.status_code = 404
action_logs_resource = ActionsStepsLogsResource()
with pytest.raises(ApiError) as e:
action_logs_resource.retrieve_logs(None)
assert ('Airflow endpoint returned error status code' in
e.value.description)
assert falcon.HTTP_404 == e.value.status
@mock.patch('requests.get')
def test_retrieve_logs_error(self, mock_get):
mock_get.side_effect = requests.exceptions.ConnectionError
action_logs_resource = ActionsStepsLogsResource()
with pytest.raises(ApiError) as e:
action_logs_resource.retrieve_logs(None)
assert ("Exception happened during Airflow API request" in
e.value.description)
assert falcon.HTTP_500 == e.value.status