#1740 Issue: fix failing Google Sheets Source with large spreadsheet #1762
Conversation
…(more than 1000 rows)
| row_batch = SpreadsheetValues.parse_obj( | ||
| client.get_values(spreadsheetId=spreadsheet_id, ranges=range, majorDimension="ROWS") | ||
| ) | ||
| try: |
There was a problem hiding this comment.
can we instead cap the range based on how large the spreadsheet is? e.g:
if row_cursor >= num_rows_in_sheet:
break
range = f"{sheet}!{row_cursor}:{min(num_rows_in_sheet, row_cursor+ROW_BATCH_SIZE)}
I think the current approach of swallowing any HttpError is not very robust as we could be getting other kinds of exceptions
There was a problem hiding this comment.
Okay, I`ll try make this
| @@ -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): | |||
There was a problem hiding this comment.
should this be called get_sheet_row_count ? Also can you add the return value type signature?
There was a problem hiding this comment.
@sherifnada done
| 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]: |
There was a problem hiding this comment.
@yevhenii-ldv wouldn't this overcount in the case that row_cusror + batchsize is greater than sheet_parameters[sheet]?
There was a problem hiding this comment.
@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.
sherifnada
deleted the
ykurochkin/google-sheets-fails-on-large-spreadsheet
branch
What
fix failing Google Sheets Source with large spreadsheet (more than 1000 rows)
How
The error occurred in cases when we try to get data from non-existing lines, which means that the file is over, we only need to catch the error and exit the loop.
One more thing, I slightly increased the maximum time for
backoff, since once when reading192302lines, error429fell after126000lines.Pre-merge Checklist
Recommended reading order
test.javacomponent.ts