pr feedback

Author: brianjlai

airbyte_cdk/sources/declarative/retrievers/simple_retriever.py

@@ -569,6 +569,12 @@ def stream_slices(self) -> Iterable[Optional[StreamSlice]]:  # type: ignore
         """
         return self.stream_slicer.stream_slices()
 
+    # todo: There are a number of things that can be cleaned up when we remove self.cursor and all the related
+    #  SimpleRetriever state management that is handled by the concurrent CDK Framework:
+    #  - ModelToComponentFactory.create_datetime_based_cursor() should be removed since it does need to be instantiated
+    #  - ModelToComponentFactory.create_incrementing_count_cursor() should be removed since it's a placeholder
+    #  - test_simple_retriever.py: Remove all imports and usages of legacy cursor components
+    #  - test_model_to_component_factory.py:test_datetime_based_cursor() test can be removed
     @property
     def state(self) -> Mapping[str, Any]:
         return self.cursor.get_stream_state() if self.cursor else {}

unit_tests/connector_builder/test_connector_builder_handler.py

@@ -7,7 +7,7 @@
 import json
 import logging
 import os
-from typing import List, Literal, Union
+from typing import List, Literal
 from unittest import mock
 from unittest.mock import MagicMock, patch
 
@@ -32,7 +32,6 @@
     StreamReadPages,
     StreamReadSlices,
 )
-from airbyte_cdk.legacy.sources.declarative.declarative_stream import DeclarativeStream
 from airbyte_cdk.models import (
     AirbyteLogMessage,
     AirbyteMessage,
@@ -440,12 +439,8 @@
 }
 
 
-def get_retriever(stream: Union[DeclarativeStream, DefaultStream]):
-    return (
-        stream.retriever
-        if isinstance(stream, DeclarativeStream)
-        else stream._stream_partition_generator._partition_factory._retriever
-    )
+def get_retriever(stream: DefaultStream):
+    return stream._stream_partition_generator._partition_factory._retriever
 
 
 @pytest.fixture
@@ -985,12 +980,6 @@ def create_mock_retriever(name, url_base, path):
     return http_stream
 
 
-def create_mock_declarative_stream(http_stream):
-    declarative_stream = mock.Mock(spec=DeclarativeStream, autospec=True)
-    declarative_stream.retriever = http_stream
-    return declarative_stream
-
-
 @pytest.mark.parametrize(
     "test_name, config, expected_max_records, expected_max_slices, expected_max_pages_per_slice",
     [

unit_tests/sources/declarative/partition_routers/test_cartesian_product_partition_router.py

@@ -22,6 +22,8 @@
 from airbyte_cdk.sources.types import StreamSlice
 
 
+# todo: All these tests rely on stream_slicers that are of a the deprecated legacy class DatetimeBasedCursor these
+#  should really be ConcurrentCursor, but this fix is a bit tedious and are tested in other parts of the code
 @pytest.mark.parametrize(
     "test_name, stream_slicers, expected_slices",
     [

unit_tests/sources/declarative/retrievers/test_simple_retriever.py

@@ -10,7 +10,6 @@
 import pytest
 import requests
 
-# todo: These can be removed once SimpleRetriever deprecated the now unused state methods
 from airbyte_cdk.legacy.sources.declarative.incremental import (
     DatetimeBasedCursor,
     DeclarativeCursor,