Skip to content

Conversation

@nagypeterjob
Copy link
Contributor

No description provided.

@nagypeterjob nagypeterjob changed the title [SVC-3061] feat: add S3 download cache functionality [SVC-3061] feat: add S3 cache functionality May 8, 2024
Comment on lines 94 to 95
matchedKey = keyFound
firstValidKey = keyFound

Choose a reason for hiding this comment

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

Cannot we drop 1 of these variables as they always have the same value?

return fmt.Errorf("creating file: %w", err)
}
defer file.Close() //nolint:errcheck
body, err := io.ReadAll(result.Body)

Choose a reason for hiding this comment

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

These objects can be quite large (gigabytes+). We should either read the body in fix sized chunks, writing to file simultaneously or use a library that does this for us (the S3 client might have such a method available, I haven't checked).

Without this, the build VM can go out of memory easily.

@szabolcsgelencser
Copy link

Nice work Peti as always! Left 1 nitpick and 1 change request, otherwise lgtm! 🙂🎉

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.

2 participants