Skip to content

Commit c581827

Browse files
Change fetch_one to raise RecordNotFoundException instead of returning None for 404s
Co-Authored-By: AJ Steers <[email protected]>
1 parent 052eb81 commit c581827

File tree

3 files changed

+33
-17
lines changed

3 files changed

+33
-17
lines changed

airbyte_cdk/sources/declarative/exceptions.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,7 @@ class ReadException(Exception):
77
"""
88
Raise when there is an error reading data from an API Source
99
"""
10+
11+
12+
class RecordNotFoundException(ReadException):
13+
"""Raised when a requested record is not found (e.g., 404 response)."""

airbyte_cdk/sources/declarative/retrievers/simple_retriever.py

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
from airbyte_cdk.legacy.sources.declarative.incremental import ResumableFullRefreshCursor
2727
from airbyte_cdk.legacy.sources.declarative.incremental.declarative_cursor import DeclarativeCursor
2828
from airbyte_cdk.models import AirbyteMessage
29+
from airbyte_cdk.sources.declarative.exceptions import RecordNotFoundException
2930
from airbyte_cdk.sources.declarative.extractors.http_selector import HttpSelector
3031
from airbyte_cdk.sources.declarative.interpolation import InterpolatedString
3132
from airbyte_cdk.sources.declarative.partition_routers.single_partition_router import (
@@ -713,14 +714,17 @@ def fetch_one(
713714
log_formatter=self.log_formatter,
714715
)
715716
except Exception as e:
716-
# Check if this is a 404 (record not found) - return None
717-
if hasattr(e, "response") and hasattr(e.response, "status_code"):
718-
if e.response.status_code == 404:
719-
return None
717+
# Check if this is a 404 (record not found) - raise RecordNotFoundException
718+
if "404" in str(e) or (hasattr(e, "response") and hasattr(e.response, "status_code") and e.response.status_code == 404):
719+
raise RecordNotFoundException(
720+
f"Record with primary key {pk_value} not found"
721+
) from e
720722
raise
721723

722724
if not response:
723-
return None
725+
raise RecordNotFoundException(
726+
f"Record with primary key {pk_value} not found (no response)"
727+
)
724728

725729
records = list(
726730
self._parse_response(
@@ -732,16 +736,20 @@ def fetch_one(
732736
)
733737
)
734738

735-
# Return the first record if found, None otherwise
739+
# Return the first record if found, raise RecordNotFoundException otherwise
736740
if records:
737741
first_record = records[0]
738742
if isinstance(first_record, Record):
739743
return dict(first_record.data)
740744
elif isinstance(first_record, Mapping):
741745
return dict(first_record)
742746
else:
743-
return None
744-
return None
747+
raise RecordNotFoundException(
748+
f"Record with primary key {pk_value} not found (invalid record type)"
749+
)
750+
raise RecordNotFoundException(
751+
f"Record with primary key {pk_value} not found (empty response)"
752+
)
745753

746754

747755
def _deep_merge(

unit_tests/sources/declarative/retrievers/test_simple_retriever.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
)
2525
from airbyte_cdk.sources.declarative.auth.declarative_authenticator import NoAuth
2626
from airbyte_cdk.sources.declarative.decoders import JsonDecoder
27+
from airbyte_cdk.sources.declarative.exceptions import RecordNotFoundException
2728
from airbyte_cdk.sources.declarative.extractors import DpathExtractor, HttpSelector, RecordSelector
2829
from airbyte_cdk.sources.declarative.partition_routers import SinglePartitionRouter
2930
from airbyte_cdk.sources.declarative.requesters.paginators import DefaultPaginator, Paginator
@@ -1719,7 +1720,7 @@ def test_fetch_one_composite_pk():
17191720

17201721

17211722
def test_fetch_one_not_found():
1722-
"""Test fetch_one returns None when record is not found (404)."""
1723+
"""Test fetch_one raises RecordNotFoundException when record is not found (404)."""
17231724
requester = MagicMock()
17241725
requester.get_path.return_value = "posts"
17251726

@@ -1739,9 +1740,10 @@ def test_fetch_one_not_found():
17391740
config={},
17401741
)
17411742

1742-
result = retriever.fetch_one("999", records_schema={})
1743+
with pytest.raises(RecordNotFoundException) as exc_info:
1744+
retriever.fetch_one("999", records_schema={})
17431745

1744-
assert result is None
1746+
assert "999" in str(exc_info.value)
17451747

17461748

17471749
def test_fetch_one_server_error():
@@ -1794,7 +1796,7 @@ def test_fetch_one_invalid_pk_type():
17941796

17951797

17961798
def test_fetch_one_no_response():
1797-
"""Test fetch_one returns None when response is None."""
1799+
"""Test fetch_one raises RecordNotFoundException when response is None."""
17981800
requester = MagicMock()
17991801
requester.get_path.return_value = "posts"
18001802
requester.send_request.return_value = None
@@ -1810,13 +1812,14 @@ def test_fetch_one_no_response():
18101812
config={},
18111813
)
18121814

1813-
result = retriever.fetch_one("123", records_schema={})
1815+
with pytest.raises(RecordNotFoundException) as exc_info:
1816+
retriever.fetch_one("123", records_schema={})
18141817

1815-
assert result is None
1818+
assert "123" in str(exc_info.value)
18161819

18171820

18181821
def test_fetch_one_empty_records():
1819-
"""Test fetch_one returns None when no records are returned."""
1822+
"""Test fetch_one raises RecordNotFoundException when no records are returned."""
18201823
requester = MagicMock()
18211824
requester.get_path.return_value = "posts"
18221825

@@ -1838,6 +1841,7 @@ def test_fetch_one_empty_records():
18381841
config={},
18391842
)
18401843

1841-
result = retriever.fetch_one("123", records_schema={})
1844+
with pytest.raises(RecordNotFoundException) as exc_info:
1845+
retriever.fetch_one("123", records_schema={})
18421846

1843-
assert result is None
1847+
assert "123" in str(exc_info.value)

0 commit comments

Comments
 (0)