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.

Sign up for GitHub

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

Jump to bottom

#1740 Issue: fix failing Google Sheets Source with large spreadsheet #1762

Merged
merged 6 commits into from
Jan 21, 2021
Merged

#1740 Issue: fix failing Google Sheets Source with large spreadsheet #1762

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f744a4c
#1740 Issue: fix failing Google Sheets Source with large spreadsheet …
yevhenii-ldv Jan 21, 2021
ec90144
#1740 Issue: update PR after review
yevhenii-ldv Jan 21, 2021
40e5c32
#1730 Issue: rename function from get_sheets_properties to get_sheet_…
yevhenii-ldv Jan 21, 2021
6c9c9af
#1730 Issue: add the return value type signature
yevhenii-ldv Jan 21, 2021
2242102
#1730 Issue: add comment to code
yevhenii-ldv Jan 21, 2021
be339d7
bump version and make test sheet size weird
sherifnada Jan 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions airbyte-integrations/connectors/source-google-sheets/google_sheets_source/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,18 @@ class GoogleSheetsClient:
def __init__(self, credentials: Dict[str, str], scopes: List[str] = SCOPES):
self.client = Helpers.get_authenticated_sheets_client(credentials, scopes)

@backoff.on_exception(backoff.expo, errors.HttpError, max_time=60, giveup=error_handler)
@backoff.on_exception(backoff.expo, errors.HttpError, max_time=120, giveup=error_handler)
def get(self, **kwargs):
return self.client.get(**kwargs).execute()

@backoff.on_exception(backoff.expo, errors.HttpError, max_time=60, giveup=error_handler)
@backoff.on_exception(backoff.expo, errors.HttpError, max_time=120, giveup=error_handler)
def create(self, **kwargs):
return self.client.create(**kwargs).execute()

@backoff.on_exception(backoff.expo, errors.HttpError, max_time=60, giveup=error_handler)
@backoff.on_exception(backoff.expo, errors.HttpError, max_time=120, giveup=error_handler)
def get_values(self, **kwargs):
return self.client.values().batchGet(**kwargs).execute()

@backoff.on_exception(backoff.expo, errors.HttpError, max_time=60, giveup=error_handler)
@backoff.on_exception(backoff.expo, errors.HttpError, max_time=120, giveup=error_handler)
def update_values(self, **kwargs):
return self.client.values().batchUpdate(**kwargs).execute()
8 changes: 4 additions & 4 deletions ...integrations/connectors/source-google-sheets/google_sheets_source/google_sheets_source.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,16 +100,18 @@ def read(
# For each sheet in the spreadsheet, get a batch of rows, and as long as there hasn't been
# a blank row, emit the row batch
sheet_to_column_index_to_name = Helpers.get_available_sheets_to_column_index_to_name(client, spreadsheet_id, sheet_to_column_name)
sheet_parameters = Helpers.get_sheets_properties(client, spreadsheet_id)
for sheet in sheet_to_column_index_to_name.keys():
logger.info(f"Syncing sheet {sheet}")
column_index_to_name = sheet_to_column_index_to_name[sheet]
row_cursor = 2 # we start syncing past the header row
while True:
while row_cursor <= sheet_parameters[sheet]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yevhenii-ldv wouldn't this overcount in the case that row_cusror + batchsize is greater than sheet_parameters[sheet]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sherifnada It is necessary that the initial row exists, if the final row of the interval will go beyond the table - that's okay, we will only return the real data of the table and in the next iteration will exit the loop.

range = f"{sheet}!{row_cursor}:{row_cursor + ROW_BATCH_SIZE}"
logger.info(f"Fetching range {range}")
row_batch = SpreadsheetValues.parse_obj(
client.get_values(spreadsheetId=spreadsheet_id, ranges=range, majorDimension="ROWS")
)

row_cursor += ROW_BATCH_SIZE + 1
# there should always be one range since we requested only one
value_ranges = row_batch.valueRanges[0]
Expand All @@ -122,8 +124,6 @@ def read(
break

for row in row_values:
if Helpers.is_row_empty(row):
continue
elif Helpers.row_contains_relevant_data(row, column_index_to_name.keys()):
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))
logger.info(f"Finished syncing spreadsheet {spreadsheet_id}")
5 changes: 5 additions & 0 deletions airbyte-integrations/connectors/source-google-sheets/google_sheets_source/helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ def get_sheets_in_spreadsheet(client, spreadsheet_id: str):
spreadsheet_metadata = Spreadsheet.parse_obj(client.get(spreadsheetId=spreadsheet_id, includeGridData=False))
return [sheet.properties.title for sheet in spreadsheet_metadata.sheets]

@staticmethod
def get_sheets_properties(client, spreadsheet_id: str):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be called get_sheet_row_count ? Also can you add the return value type signature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sherifnada done

spreadsheet_metadata = Spreadsheet.parse_obj(client.get(spreadsheetId=spreadsheet_id, includeGridData=False))
return {sheet.properties.title: sheet.properties.gridProperties["rowCount"] for sheet in spreadsheet_metadata.sheets}

@staticmethod
def is_row_empty(cell_values: List[str]) -> bool:
for cell in cell_values:
Expand Down