Skip to content
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

Merged
merged 5 commits into from
Jun 4, 2024

Conversation

davydov-d
Copy link
Collaborator

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?

  • YES 💚
  • NO ❌

Copy link

vercel bot commented Jun 2, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 3, 2024 6:18pm

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/google-sheets labels Jun 2, 2024
Comment on lines +210 to +216
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)
Copy link
Collaborator

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

Copy link
Collaborator Author

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

@davydov-d davydov-d merged commit 029d7c3 into master Jun 4, 2024
31 checks passed
@davydov-d davydov-d deleted the ddavydov/source-google-sheets-emit-state-msg branch June 4, 2024 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/google-sheets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants