-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Description
Summary
What change needs making?
Currently, if the path has /
suffix,
we attempt to download the file first then failed and try download the whole dir,
if the path has /
suffix, we should download the dir straight away, instead of wasting 1 request to download the file
argo-workflows/workflow/artifacts/s3/s3.go
Lines 191 to 212 in 4b2576f
func loadS3Artifact(ctx context.Context, s3cli S3Client, inputArtifact *wfv1.Artifact, path string) (bool, error) { | |
origErr := s3cli.GetFile(inputArtifact.S3.Bucket, inputArtifact.S3.Key, path) | |
if origErr == nil { | |
return true, nil | |
} | |
if !IsS3ErrCode(origErr, "NoSuchKey") { | |
return !isTransientS3Err(ctx, origErr), fmt.Errorf("failed to get file: %v", origErr) | |
} | |
// If we get here, the error was a NoSuchKey. The key might be an s3 "directory" | |
isDir, err := s3cli.IsDirectory(inputArtifact.S3.Bucket, inputArtifact.S3.Key) | |
if err != nil { | |
return !isTransientS3Err(ctx, err), fmt.Errorf("failed to test if %s is a directory: %v", inputArtifact.S3.Key, err) | |
} | |
if !isDir { | |
// It's neither a file, nor a directory. Return the original NoSuchKey error | |
return true, argoerrs.New(argoerrs.CodeNotFound, origErr.Error()) | |
} | |
if err = s3cli.GetDirectory(inputArtifact.S3.Bucket, inputArtifact.S3.Key, path); err != nil { | |
return !isTransientS3Err(ctx, err), fmt.Errorf("failed to get directory: %v", err) | |
} | |
return true, nil |
Use Cases
When would you use this?
Message from the maintainers:
Love this feature request? Give it a 👍. We prioritise the proposals with the most 👍.