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

Fix cases where the datafile is huge #360

Merged
merged 3 commits into from
Feb 15, 2024
Merged

Fix cases where the datafile is huge #360

merged 3 commits into from
Feb 15, 2024

Conversation

ewjoachim
Copy link
Member

@ewjoachim ewjoachim commented Feb 15, 2024

When the json datafile is >1MB, the default mode of the Github content API is to skip the content and provide a link to it.

If we use header Accept: application/vnd.github.raw+json, the application will:

  • Return the raw file instead of a payload (it's good, we only want the file) as long as it's <100MB
  • Return Content-Type: application/vnd.github.raw+json

This PR:

  • Adds code to the GitHub client to send custom headers (surprisingly, it's the first time we needed to do it)
  • Add the said Accept header to the datafile function
  • Setup the client so that the said Content-Type response header makes return text instead of bytes. (I'm not 100% sure it's what we should do but we can always revert this later).

Copy link

End-to-end public repo

Admin commands cheatsheet:

  • /e2e (in approved PR review body): Trigger end-to-end tests on external contributions
  • /invite (in comment): Invite the author & admins to the end-to-end private repo

Copy link

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  coverage_comment
  github_client.py
  storage.py
Project Total  

This report was generated by python-coverage-comment-action

@ewjoachim ewjoachim merged commit b16205b into main Feb 15, 2024
2 checks passed
@ewjoachim ewjoachim deleted the huge-datafile branch February 15, 2024 21:13
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