这是indexloc提供的服务，不要输入任何密码
Skip to content

Explicitly read the artifact into memory before uploading to the remote cache api #898

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
Merged
merged 10 commits into from
Mar 21, 2022
Merged

Explicitly read the artifact into memory before uploading to the remote cache api #898

merged 10 commits into from
Mar 21, 2022

Conversation

gaspar09
Copy link
Contributor

Read the tar artifact into memory so that we can compute the content-md5
This read was already happening implicitly via the retryableHttp module.

@gaspar09
Read the tar artifact into memory so that we can compute the content-md5
37e02cc 
Since the artifact is already read anyways, this doens't add additional overhead.
@vercel
Copy link

vercel bot commented Mar 17, 2022 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/vercel/turbo-site/5PpWuHkj1vBhHvKQHRx1n4dqGgC1
✅ Preview: https://turbo-site-git-gaspar-read-artifact-in-memory-to-compute-md5.vercel.sh

[Deployment for a50134d failed]

@gsoltis
Copy link
Contributor

gsoltis commented Mar 17, 2022

@gaspar09 What do you think about setting the Content-MD5 as a trailer rather than header to verify that we will eventually be able to use that if do want to do streaming?

@gaspar09
Copy link
Contributor Author

@gaspar09 What do you think about setting the Content-MD5 as a trailer rather than header to verify that we will eventually be able to use that if do want to do streaming?

Yep. Going to add a test to make sure it actually gets added as expected.

gaspar09
gaspar09 commented Mar 18, 2022
View reviewed changes
gsoltis
gsoltis previously approved these changes Mar 18, 2022
View reviewed changes
@gaspar09 gaspar09 added the pr: automerge Kodiak will merge these automatically after checks pass label Mar 18, 2022
@jaredpalmer
Copy link
Contributor

jaredpalmer commented Mar 18, 2022 •
edited
Loading

Let's change the global cache key here to be safe. @gaspar09 your turn to pick

@gaspar09 gaspar09 removed the pr: automerge Kodiak will merge these automatically after checks pass label Mar 18, 2022
@kodiakhq kodiakhq bot merged commit fbba134 into main Mar 21, 2022
@kodiakhq kodiakhq bot deleted the gaspar/read-artifact-in-memory-to-compute-md5 branch March 21, 2022 19:52
gaspar09 added a commit that referenced this pull request Mar 21, 2022
@gaspar09
Revert "Explicitly read the artifact into memory before uploading to …
304686a 
…the remote cache api (#898)"

This reverts commit fbba134.
@gaspar09 gaspar09 mentioned this pull request Mar 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsoltis gsoltis gsoltis approved these changes

@jaredpalmer jaredpalmer Awaiting requested review from jaredpalmer

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gaspar09 @gsoltis @jaredpalmer