Skip to content

fix: load_partial_csv fallback in Google Sheet imports wraps response in DictReader - #546

Open
ayushshukla1807 wants to merge 1 commit into
hatnote:masterfrom
ayushshukla1807:fix/load-partial-csv-gsheet-fallback
Open

fix: load_partial_csv fallback in Google Sheet imports wraps response in DictReader#546
ayushshukla1807 wants to merge 1 commit into
hatnote:masterfrom
ayushshukla1807:fix/load-partial-csv-gsheet-fallback

Conversation

@ayushshukla1807

Copy link
Copy Markdown
Contributor

Fixes #517.

The fallback path in get_entries_from_gsheet called load_partial_csv(resp) passing the raw requests.Response object directly. The function expects a DictReader (it immediately does [r["filename"] for r in dr]), so this always raised a TypeError and permanently fell through to the last-resort filename-splitting path.

The TODO comment in the code even acknowledged this: # TODO: load_partial_csv expects a dictreader, did this ever work?

Fix: Wrap resp.content in BytesIO and pass it through unicodecsv.DictReader before calling load_partial_csv. The existing except (ValueError, TypeError) fallback is preserved for any subsequent failures.

@ItsAbhinavM ItsAbhinavM left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for this @ayushshukla1807. This correctly fixes the resp issue with DictReader, but #517 also asks that the broad except (ValueError, TypeError ): around this fallback be removed so failures surface instead of being silently redirected to load_name_list. I think your other PR #521 handles both parts properly. @lgelauff I think we can close this PR. Appreciate you digging into this either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistency: load_partial_csv fallback permanently fails during Google Sheet imports

2 participants