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:
parent
12d79ce423
commit
0e6486be8b
doc/source
src/bin/shipyard_airflow
shipyard_airflow/control/action
tests/unit/control
@ -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
|
||||
'''''''
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
Loading…
x
Reference in New Issue
Block a user