-
Notifications
You must be signed in to change notification settings - Fork 117
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
Add support for @aws-sdk/client-s3 #277
base: master
Are you sure you want to change the base?
Conversation
refactor: use promise to resolve stream
else | ||
resolve(d.ContentLength); | ||
if (isAwsSdkV3) { | ||
return client.headObject(params).then(function (res) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But AWS SDK V3 doesn't have S3Client doesn't have the said methods, S3 class does, though, it doesn't have the headObject
function?send
method, hence, the PR still breaks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, yea I gave this a try and it doesn't seem to work out. What if you did something like
try {
return client.headObject(params).promise()
} catch(e) {
if (e.msg === 'something') {
return client.headObject(params).then()
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alvin-nt thanks for opening this PR, do you have the bandwidth to add this tweak or should I give it a shot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi. Apologies for the long reply. Due to life circumstances, I could not continue this PR yet. If you may, please give a shot to fix my code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be enough to change isAwsSdkV3
to this:
const isAwsSdkV3 = typeof client.send === "function";
Here, at least, it gives true
with v3 and false
with v2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, Maybe we can do something like:
if (isAwsSdkV3) {
return client.send(new HeadObjectCommand(params)).then(function (res) {
return res.ContentLength;
});
}
Thanks @alvin-nt and sorry for the long delay. I'm being more active in managing this repo and your PR is now deviating from master. Can you rebase and clean it up so it's only reflecting the s3 change and no other changes Thanks again! |
This is an attempt to add support for @aws-sdk/client-s3 (AWS SDK v3). If merged, this PR should resolve #241