-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
🐛 Source Google Sheets: emit state message #38851
🐛 Source Google Sheets: emit state message #38851
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
airbyte-integrations/connectors/source-google-sheets/source_google_sheets/source.py
Outdated
Show resolved
Hide resolved
yield as_airbyte_message(stream, AirbyteStreamStatus.RUNNING) | ||
for row in row_values: | ||
if not Helpers.is_row_empty(row) and Helpers.row_contains_relevant_data(row, column_index_to_name.keys()): | ||
yield AirbyteMessage(type=Type.RECORD, record=Helpers.row_data_to_record_message(sheet, row, column_index_to_name)) | ||
|
||
yield self._checkpoint_state(checkpoint_reader.get_checkpoint(), state_manager, sheet, None) | ||
yield as_airbyte_message(stream, AirbyteStreamStatus.COMPLETE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add error handling for per-stream error, like it is implemented in class AbstractSource
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it affect emitting a state message? I'd rather implement it in a separate ticket if it does not
….com:airbytehq/airbyte into ddavydov/source-google-sheets-emit-state-msg
What
https://github.com/airbytehq/airbyte-internal-issues/issues/7425
How
Emit a state message at least once per stream
User Impact
No negative impact expected
Can this PR be safely reverted and rolled back?