Add GCS and S3 using GO Cloud Dev #70
Conversation
khrm
commented
Oct 7, 2024
•
khrm
commented
- This removes the need to maintain the configuration for individual cloud providers.
- Refactor OCI code to remove redundant replace.
- Added Unit Test for Upload and Fetch for cloud provider.
|
@khrm sorry, needs a rebase 🤦🏼 |
khrm
force-pushed
the
blob
branch
3 times, most recently
from
578177a to
451bd58
Compare
khrm
force-pushed
the
blob
branch
3 times, most recently
from
a6d453f to
f8c149c
Compare
|
@vdemeester Updated this. |
|
After the call we decided to merge this. |
internal/provider/blob/blob.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| defer clean() //nolint:errcheck |
There was a problem hiding this comment.
you don't have to ignore that error if you want to
There was a problem hiding this comment.
OK. So I am just logging if some error happens.
| return err | ||
| } | ||
|
|
||
| if err := tar.Untar(ctx, file, folder); err != nil { |
There was a problem hiding this comment.
i wonder if we can optimize the reading of the object and unar it to the same step without having to go by a temporary file locally.... this would greatly optimize it with a large cache, wdyt ?
There was a problem hiding this comment.
We are compressing it, not only taring it. That function name should be changed. Maybe we can do it in a follow up PR
There was a problem hiding this comment.
sounds good, can you create an issue for tracking?
There was a problem hiding this comment.
BTW, I change from io.Readall to io.copy. Won't Readall give OOM?
There was a problem hiding this comment.
Created an issue to track this.
There was a problem hiding this comment.
yeah io.Copy stream instead of reading everything at once in memory like ReadAll but that's beside the point that we can stream directly to gzip/upload to bucket.
