Skip to content

Commit 93f83ca

Browse files
committed
pr feedback to remove QueryProperty method and fix a test
1 parent 63062d9 commit 93f83ca

File tree

4 files changed

+18
-56
lines changed

4 files changed

+18
-56
lines changed

airbyte_cdk/sources/declarative/requesters/query_properties/query_properties.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ def get_request_property_chunks(
4141
else:
4242
yield list(fields)
4343

44+
# delete later, but leaving this to keep the discussion thread on the PR from getting hidden
4445
def has_multiple_chunks(self, stream_slice: Optional[StreamSlice]) -> bool:
4546
property_chunks = iter(self.get_request_property_chunks(stream_slice=stream_slice))
4647
try:

airbyte_cdk/sources/declarative/retrievers/simple_retriever.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -456,9 +456,7 @@ def read_records(
456456
stream_slice=stream_slice
457457
)
458458
)
459-
has_multiple_chunks = self.additional_query_properties.has_multiple_chunks(
460-
stream_slice=stream_slice
461-
)
459+
has_multiple_chunks = self._has_multiple_chunks(stream_slice=stream_slice)
462460
else:
463461
property_chunks = [[""]]
464462
has_multiple_chunks = False
@@ -598,6 +596,20 @@ def _extract_record(
598596
)
599597
return None
600598

599+
def _has_multiple_chunks(self, stream_slice: Optional[StreamSlice]) -> bool:
600+
if not self.additional_query_properties:
601+
return False
602+
603+
property_chunks = iter(
604+
self.additional_query_properties.get_request_property_chunks(stream_slice=stream_slice)
605+
)
606+
try:
607+
next(property_chunks)
608+
next(property_chunks)
609+
return True
610+
except StopIteration:
611+
return False
612+
601613
# stream_slices is defined with arguments on http stream and fixing this has a long tail of dependencies. Will be resolved by the decoupling of http stream and simple retriever
602614
def stream_slices(self) -> Iterable[Optional[StreamSlice]]: # type: ignore
603615
"""

unit_tests/sources/declarative/parsers/test_model_to_component_factory.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import freezegun
1010
import pytest
1111
import requests
12+
from pydantic.v1 import ValidationError
1213

1314
from airbyte_cdk import AirbyteTracedException
1415
from airbyte_cdk.models import (
@@ -161,9 +162,7 @@
161162
ClampingEndProvider,
162163
DayClampingStrategy,
163164
MonthClampingStrategy,
164-
NoClamping,
165165
WeekClampingStrategy,
166-
Weekday,
167166
)
168167
from airbyte_cdk.sources.streams.concurrent.cursor import ConcurrentCursor
169168
from airbyte_cdk.sources.streams.concurrent.state_converters.datetime_stream_state_converter import (
@@ -4375,7 +4374,7 @@ def test_create_property_chunking_invalid_property_limit_type():
43754374

43764375
connector_builder_factory = ModelToComponentFactory(emit_connector_builder_messages=True)
43774376

4378-
with pytest.raises:
4377+
with pytest.raises(ValidationError):
43794378
connector_builder_factory.create_component(
43804379
model_type=PropertyChunkingModel,
43814380
component_definition=property_chunking_model,

unit_tests/sources/declarative/requesters/query_properties/test_query_properties.py

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22

33
from unittest.mock import Mock
44

5-
import pytest
6-
75
from airbyte_cdk.sources.declarative.requesters.query_properties import (
86
PropertiesFromEndpoint,
97
QueryProperties,
@@ -115,51 +113,3 @@ def test_get_request_property_chunks_dynamic_endpoint():
115113
assert len(property_chunks) == 2
116114
assert property_chunks[0] == ["alice", "clover", "dio", "k", "luna"]
117115
assert property_chunks[1] == ["phi", "quark", "sigma", "tenmyouji"]
118-
119-
120-
@pytest.mark.parametrize(
121-
"property_limit,expected_has_multiple_chunks",
122-
[
123-
pytest.param(
124-
5,
125-
True,
126-
id="test_has_multiple_chunks",
127-
),
128-
pytest.param(
129-
10,
130-
False,
131-
id="test_has_multiple_chunks",
132-
),
133-
],
134-
)
135-
def test_has_multiple_chunks(property_limit, expected_has_multiple_chunks):
136-
stream_slice = StreamSlice(cursor_slice={}, partition={})
137-
138-
query_properties = QueryProperties(
139-
property_list=[
140-
"ace",
141-
"snake",
142-
"santa",
143-
"clover",
144-
"junpei",
145-
"june",
146-
"seven",
147-
"lotus",
148-
"nine",
149-
],
150-
always_include_properties=None,
151-
property_chunking=PropertyChunking(
152-
property_limit_type=PropertyLimitType.property_count,
153-
property_limit=property_limit,
154-
record_merge_strategy=GroupByKey(key="id", config=CONFIG, parameters={}),
155-
config=CONFIG,
156-
parameters={},
157-
),
158-
config=CONFIG,
159-
parameters={},
160-
)
161-
162-
assert (
163-
query_properties.has_multiple_chunks(stream_slice=stream_slice)
164-
== expected_has_multiple_chunks
165-
)

0 commit comments

Comments
 (0)