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

Reading: Update GoogleSheetsFetcher to recur into folders and batch calls #2623

Merged
merged 4 commits into from
Sep 25, 2019

Conversation

kevinrobinson
Copy link
Contributor

@kevinrobinson kevinrobinson commented Sep 25, 2019

Who is this PR for?

Somerville K5 students, reading specialists, literacy coaches

What problem does this PR fix?

Previous PRs like #2619 #2620 #2621 worked out the kinks running the importer task over individual schools, but running it over the whole district ran into a few issues related to fetching data. First, #get_tabs_from_folder assumed that all files within a folder were sheets, but turns out this isn't always the case. Second, it seems that the in parent query in Google Drive searches only within the immediate "children" of a folder and not recursively which I think I was assuming. Third, when updating to search recursively, we hit API request quotas.

What does this PR do?

  1. Updates the methods making API calls to explicitly limit results to files that are sheets or folders.
  2. Adds a recursive: true option to #get_tabs_from_folder.
  3. Uses a batch method to get values from all sheets in a spreadsheet in one request.
  4. Handle the special case of the API returning nil when calling #values on a newly added sheet.
  5. Add optional logging that was useful in debugging.

Does this PR use tests to help verify we can deploy these changes quickly and confidently?

  • Manual testing made more sense here

@kevinrobinson
Copy link
Contributor Author

@edavidsonsawyer This fetcher class is getting some good mileage! :) I'm going to merge this now, but it'd be awesome if you could review and share any comments or feedback whenever you have a chance.

@kevinrobinson kevinrobinson merged commit 0919c70 into master Sep 25, 2019
@kevinrobinson kevinrobinson deleted the patch/google-sheets-fetcher-optimized branch September 25, 2019 14:43
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.

1 participant