Connector builder: read input state if it exists #37495
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
girarda
changed the title
| ) -> Iterator[AirbyteMessage]: | ||
| # the generator can raise an exception | ||
| # iterate over the generated messages. if next raise an exception, catch it and yield it as an AirbyteLogMessage | ||
| try: | ||
| yield from AirbyteEntrypoint(source).read(source.spec(self.logger), config, configured_catalog, {}) | ||
| yield from AirbyteEntrypoint(source).read(source.spec(self.logger), config, configured_catalog, state) |
There was a problem hiding this comment.
the input state used to be hardcoded to an empty legacy state object
…te into alex/builder_incremental
| @@ -457,7 +459,7 @@ def test_read(): | |||
| limits = TestReadLimits() | |||
| with patch("airbyte_cdk.connector_builder.message_grouper.MessageGrouper.get_message_groups", return_value=stream_read): | |||
| output_record = handle_connector_builder_request( | |||
| source, "test_read", config, ConfiguredAirbyteCatalog.parse_obj(CONFIGURED_CATALOG), limits | |||
| source, "test_read", config, ConfiguredAirbyteCatalog.parse_obj(CONFIGURED_CATALOG), _NO_STATE, limits | |||
There was a problem hiding this comment.
Should we add a test here that actually sets some state, to make sure that is handled properly?
There was a problem hiding this comment.
good idea. Updated the input state and added an assertion https://github.com/airbytehq/airbyte/pull/37495/files#diff-f655e63774ac8a9d9b3b6f37f553a5e726643dc5da46d39ab11e879775b8a3edR477
|
|
||
|
|
||
| def handle_connector_builder_request( | ||
| source: ManifestDeclarativeSource, | ||
| command: str, | ||
| config: Mapping[str, Any], | ||
| catalog: Optional[ConfiguredAirbyteCatalog], | ||
| state: List[AirbyteStateMessage], |
There was a problem hiding this comment.
I asked the same question over slack, but asking here too for posterity: it seems like the connection state UI shows a list of AirbyteStreamState objects, whereas this method takes in a list of AirbyteStateMessages.
Which one is right? Ideally these would be consistent so anyone could copy the state from the connection UI and paste it into the builder's state input
There was a problem hiding this comment.
As discussed on slack:
- we'll use
AirbyteStateMessages in the builder because they contain more information - we won't couple this with the platform view. We'll brainstorm novel solutions if the need to paste configs from existing connections really is a need
What
Updates the connector builder to read an optional input state and pass it to the Entrypoint's read command.
This enables the test_read endpoint to read incrementally.
Demo with the builder: https://www.loom.com/share/169c02e5b9e04723ad5b048aa616c27a
Can this PR be safely reverted and rolled back?