Google Sheets Destination flushes on every state message and flush when buffer gets too large #14751

evantahler · 2022-07-15T17:15:33Z

This PR flushes any pending RECORD writes to the Google Sheet whenever the destination sees a STATE message. This fixes a broken spec implementation and should lead to lower memory consumption as well.

evantahler · 2022-07-15T17:31:13Z

/test connector=connectors/destination-google_sheets

🕑 connectors/destination-google_sheets https://github.com/airbytehq/airbyte/actions/runs/2678366594
❌ connectors/destination-google_sheets https://github.com/airbytehq/airbyte/actions/runs/2678366594
🐛

Build Failed

Test summary info:

Could not find result summary

evantahler · 2022-07-15T17:31:28Z

/test connector=connectors/destination-google-sheets

🕑 connectors/destination-google-sheets https://github.com/airbytehq/airbyte/actions/runs/2678367697
❌ connectors/destination-google-sheets https://github.com/airbytehq/airbyte/actions/runs/2678367697
🐛 https://gradle.com/s/64etrvp45rvyu

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 ERROR integration_tests/test_client.py - google.auth.exceptions.RefreshError:...
	 ERROR integration_tests/test_destination.py - google.auth.exceptions.RefreshE...
	 ERROR integration_tests/test_helpers.py - google.auth.exceptions.RefreshError...
	 ERROR integration_tests/test_spreadsheet.py - google.auth.exceptions.RefreshE...
	 ERROR integration_tests/test_writer.py - google.auth.exceptions.RefreshError:...
	 !!!!!!!!!!!!!!!!!!! Interrupted: 5 errors during collection !!!!!!!!!!!!!!!!!!!!
	 �[31m======================== �[33m3 warnings�[0m, �[31m�[1m5 errors�[0m�[31m in 2.21s�[0m�[31m =========================�[0m

evantahler · 2022-07-15T17:37:24Z

Note for the future - the directory structure doesn't match the gradle names #14751 (comment)

evantahler · 2022-07-15T17:39:45Z

The test failures are realy about permissions:

E   google.auth.exceptions.RefreshError: ('invalid_grant: Bad Request', ***'error': 'invalid_grant', 'error_description': 'Bad Request'***)

@sherifnada or @bazarnov: any tips to running these tests?

bazarnov · 2022-07-15T19:22:44Z

Note for the future - the directory structure doesn't match the gradle names #14751 (comment)

What exactly should match?

bazarnov · 2022-07-15T19:26:58Z

@sherifnada or @bazarnov: any tips to running these tests?

Looks like it didn't recognise the secret, all tests are passed locally just now see the screenshot:

bazarnov · 2022-07-15T19:27:22Z

/test connector=connectors/destination-google-sheets

🕑 connectors/destination-google-sheets https://github.com/airbytehq/airbyte/actions/runs/2678921580
✅ connectors/destination-google-sheets https://github.com/airbytehq/airbyte/actions/runs/2678921580
Python tests coverage:

Name                                       Stmts   Miss  Cover
--------------------------------------------------------------
destination_google_sheets/buffer.py           27      0   100%
destination_google_sheets/__init__.py          2      0   100%
destination_google_sheets/writer.py           41      1    98%
destination_google_sheets/client.py           25      1    96%
destination_google_sheets/spreadsheet.py      46      2    96%
destination_google_sheets/helpers.py          47      4    91%
destination_google_sheets/destination.py      47      9    81%
--------------------------------------------------------------
TOTAL                                        235     17    93%

Build Passed

Test summary info:

All Passed

...e-integrations/connectors/destination-google-sheets/destination_google_sheets/destination.py

airbyte-integrations/connectors/destination-google-sheets/destination_google_sheets/buffer.py

evantahler · 2022-07-15T19:44:49Z

Thanks for fixing the test @bazarnov, but I don't quite understand your logical change

bazarnov · 2022-07-15T19:54:32Z

Thanks for fixing the test @bazarnov, but I don't quite understand your logical change

There was no change), I've simply updated the GSM secret to the one I've got locally, probably I forgot to update it after the successful /publish the initial version. (There was another link for the google sheet I believe, which was not accessible for anyone)

evantahler · 2022-07-15T20:33:02Z

/test connector=connectors/destination-google-sheets

🕑 connectors/destination-google-sheets https://github.com/airbytehq/airbyte/actions/runs/2679211386
✅ connectors/destination-google-sheets https://github.com/airbytehq/airbyte/actions/runs/2679211386
Python tests coverage:

Name                                       Stmts   Miss  Cover
--------------------------------------------------------------
destination_google_sheets/buffer.py           27      0   100%
destination_google_sheets/__init__.py          2      0   100%
destination_google_sheets/writer.py           41      1    98%
destination_google_sheets/client.py           25      1    96%
destination_google_sheets/spreadsheet.py      46      2    96%
destination_google_sheets/helpers.py          47      4    91%
destination_google_sheets/destination.py      47      9    81%
--------------------------------------------------------------
TOTAL                                        235     17    93%

Build Passed

Test summary info:

All Passed

bazarnov · 2022-07-15T21:05:17Z

@sherifnada @evantahler
I've added some simple check for the size of the records_buffer(target_stream) in addition to flush_interval check, before we write records to the destination. Now, when the size of the buffer for particular stream being processed is more than flush_interval_size_in_kb we will flush the records to the destination and clean the buffer. Please adjust the value of flush_interval_size_in_kb to reflect your best practices to handle OOM issues once needed.

bazarnov · 2022-07-15T21:07:16Z

/test connector=connectors/destination-google-sheets

🕑 connectors/destination-google-sheets https://github.com/airbytehq/airbyte/actions/runs/2679364329
✅ connectors/destination-google-sheets https://github.com/airbytehq/airbyte/actions/runs/2679364329
Python tests coverage:

Name                                       Stmts   Miss  Cover
--------------------------------------------------------------
destination_google_sheets/buffer.py           28      0   100%
destination_google_sheets/__init__.py          2      0   100%
destination_google_sheets/writer.py           42      1    98%
destination_google_sheets/client.py           25      1    96%
destination_google_sheets/spreadsheet.py      46      2    96%
destination_google_sheets/helpers.py          47      4    91%
destination_google_sheets/destination.py      47      9    81%
--------------------------------------------------------------
TOTAL                                        237     17    93%

Build Passed

Test summary info:

All Passed

evantahler · 2022-07-15T21:43:06Z

That's great @bazarnov! If you review this PR, I'll get it published.

sherifnada

thanks to you both!

airbyte-integrations/connectors/destination-google-sheets/destination_google_sheets/writer.py

…ination_google_sheets/writer.py Co-authored-by: Sherif A. Nada <[email protected]>

evantahler · 2022-07-15T22:08:17Z

/publish connector=connectors/destination-google-sheets

🕑 Publishing the following connectors:
connectors/destination-google-sheets
https://github.com/airbytehq/airbyte/actions/runs/2679610819

Connector	Did it publish?	Were definitions generated?
connectors/destination-google-sheets	✅	✅

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

Google Sheets Destination flushes on every state message

8ede1ba

github-actions bot added area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Jul 15, 2022

octavia-squidington-iii added the connectors/destination/google-sheets label Jul 15, 2022

evantahler temporarily deployed to more-secrets July 15, 2022 17:17 Inactive

add PR number to readme

a7e2e9b

evantahler temporarily deployed to more-secrets July 15, 2022 17:21 Inactive

Merge branch 'master' into evan/google-sheet-destination-flush-better

49294ed

evantahler temporarily deployed to more-secrets July 15, 2022 18:43 Inactive

suggestions

75d1d4f

bazarnov temporarily deployed to more-secrets July 15, 2022 19:23 Inactive

evantahler commented Jul 15, 2022

View reviewed changes

...e-integrations/connectors/destination-google-sheets/destination_google_sheets/destination.py Outdated Show resolved Hide resolved

airbyte-integrations/connectors/destination-google-sheets/destination_google_sheets/buffer.py Outdated Show resolved Hide resolved

reverted clean change

c3fd77d

bazarnov temporarily deployed to more-secrets July 15, 2022 19:54 Inactive

evantahler marked this pull request as ready for review July 15, 2022 20:03

evantahler requested review from bazarnov and sherifnada July 15, 2022 20:04

bazarnov temporarily deployed to more-secrets July 15, 2022 20:32 Inactive

evantahler mentioned this pull request Jul 15, 2022

Destinations emit state only once RECORDS make it to _airbyte_raw_* tables #14768

Closed

added memory allocation check for records_buffer(stream)

9a21e9c

bazarnov temporarily deployed to more-secrets July 15, 2022 21:03 Inactive

evantahler changed the title ~~Google Sheets Destination flushes on every state message~~ Google Sheets Destination flushes on every state message and flush when buffer gets too large Jul 15, 2022

sherifnada approved these changes Jul 15, 2022

View reviewed changes

airbyte-integrations/connectors/destination-google-sheets/destination_google_sheets/writer.py Show resolved Hide resolved

Update airbyte-integrations/connectors/destination-google-sheets/dest…

174af0a

…ination_google_sheets/writer.py Co-authored-by: Sherif A. Nada <[email protected]>

evantahler temporarily deployed to more-secrets July 15, 2022 21:48 Inactive

auto-bump connector version

ececb74

octavia-squidington-iii temporarily deployed to more-secrets July 15, 2022 22:25 Inactive

evantahler merged commit b0f559d into master Jul 15, 2022

evantahler deleted the evan/google-sheet-destination-flush-better branch July 15, 2022 23:15

octavia-squidington-iii mentioned this pull request Jul 23, 2022

Bump Airbyte version from 0.39.37-alpha to 0.39.38-alpha #14976

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Google Sheets Destination flushes on every state message and flush when buffer gets too large #14751

Google Sheets Destination flushes on every state message and flush when buffer gets too large #14751

evantahler commented Jul 15, 2022 •

edited

Loading

evantahler commented Jul 15, 2022 •

edited by github-actions bot

Loading

evantahler commented Jul 15, 2022 •

edited by github-actions bot

Loading

evantahler commented Jul 15, 2022

evantahler commented Jul 15, 2022

bazarnov commented Jul 15, 2022

bazarnov commented Jul 15, 2022 •

edited

Loading

bazarnov commented Jul 15, 2022 •

edited by github-actions bot

Loading

evantahler commented Jul 15, 2022

bazarnov commented Jul 15, 2022

evantahler commented Jul 15, 2022 •

edited by github-actions bot

Loading

bazarnov commented Jul 15, 2022 •

edited

Loading

bazarnov commented Jul 15, 2022 •

edited by github-actions bot

Loading

evantahler commented Jul 15, 2022

sherifnada left a comment

evantahler commented Jul 15, 2022 •

edited by github-actions bot

Loading

Google Sheets Destination flushes on every state message and flush when buffer gets too large #14751

Google Sheets Destination flushes on every state message and flush when buffer gets too large #14751

Conversation

evantahler commented Jul 15, 2022 • edited Loading

evantahler commented Jul 15, 2022 • edited by github-actions bot Loading

Build Failed

evantahler commented Jul 15, 2022 • edited by github-actions bot Loading

Build Failed

evantahler commented Jul 15, 2022

evantahler commented Jul 15, 2022

bazarnov commented Jul 15, 2022

bazarnov commented Jul 15, 2022 • edited Loading

bazarnov commented Jul 15, 2022 • edited by github-actions bot Loading

Build Passed

evantahler commented Jul 15, 2022

bazarnov commented Jul 15, 2022

evantahler commented Jul 15, 2022 • edited by github-actions bot Loading

Build Passed

bazarnov commented Jul 15, 2022 • edited Loading

bazarnov commented Jul 15, 2022 • edited by github-actions bot Loading

Build Passed

evantahler commented Jul 15, 2022

sherifnada left a comment

Choose a reason for hiding this comment

evantahler commented Jul 15, 2022 • edited by github-actions bot Loading

evantahler commented Jul 15, 2022 •

edited

Loading

evantahler commented Jul 15, 2022 •

edited by github-actions bot

Loading

evantahler commented Jul 15, 2022 •

edited by github-actions bot

Loading

bazarnov commented Jul 15, 2022 •

edited

Loading

bazarnov commented Jul 15, 2022 •

edited by github-actions bot

Loading

evantahler commented Jul 15, 2022 •

edited by github-actions bot

Loading

bazarnov commented Jul 15, 2022 •

edited

Loading

bazarnov commented Jul 15, 2022 •

edited by github-actions bot

Loading

evantahler commented Jul 15, 2022 •

edited by github-actions bot

Loading