Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[low-code connectors] Handle 200 responses with error #15055

Merged
merged 2 commits into from
Jul 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#
# Copyright (c) 2022 Airbyte, Inc., all rights reserved.
#
class ReadException(Exception):
"""
Raise when there is an error reading data from an API Source
"""
Original file line number Diff line number Diff line change
Expand Up @@ -112,20 +112,20 @@ def max_retries(self) -> Union[int, None]:

def should_retry(self, response: requests.Response) -> ResponseStatus:
request = response.request
if response.ok:
return response_status.SUCCESS

if request not in self._last_request_to_attempt_count:
self._last_request_to_attempt_count = {request: 1}
else:
self._last_request_to_attempt_count[request] += 1

for response_filter in self._response_filters:
filter_action = response_filter.matches(response)
if filter_action is not None:
if filter_action == ResponseAction.RETRY:
return ResponseStatus(ResponseAction.RETRY, self._backoff_time(response, self._last_request_to_attempt_count[request]))
else:
return ResponseStatus(filter_action)
if response.ok:
return response_status.SUCCESS
# Fail if the response matches no filters
return response_status.FAIL

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,5 @@ def _response_contains_error_message(self, response: requests.Response) -> bool:
if not self._error_message_contains:
return False
else:
return self._error_message_contains in HttpStream.parse_response_error_message(response)
error_message = HttpStream.parse_response_error_message(response)
return error_message and self._error_message_contains in error_message
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import requests
from airbyte_cdk.models import SyncMode
from airbyte_cdk.sources.declarative.extractors.http_selector import HttpSelector
from airbyte_cdk.sources.declarative.read_exception import ReadException
from airbyte_cdk.sources.declarative.requesters.error_handlers.response_action import ResponseAction
from airbyte_cdk.sources.declarative.requesters.paginators.no_pagination import NoPagination
from airbyte_cdk.sources.declarative.requesters.paginators.paginator import Paginator
Expand Down Expand Up @@ -263,7 +264,7 @@ def parse_response(
# else -> delegate to record selector
response_status = self._requester.should_retry(response)
if response_status.action == ResponseAction.FAIL:
response.raise_for_status()
raise ReadException(f"Request {response.request} failed with response {response}")
elif response_status.action == ResponseAction.IGNORE:
self.logger.info(f"Ignoring response for failed request with error message {HttpStream.parse_response_error_message(response)}")
return []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@
response_status.FAIL,
None,
),
(
"test_200_fail_with_predicate",
HTTPStatus.OK,
HttpResponseFilter(action=ResponseAction.FAIL, error_message_contain="found"),
None,
{},
response_status.FAIL,
None,
),
(
"test_retry_403",
HTTPStatus.FORBIDDEN,
Expand Down