Skip to content
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

feat: [s3] implementation of api requests around s3 multipart upload #42

Merged
merged 4 commits into from
May 2, 2023

Conversation

ryoshindo
Copy link
Contributor

Hello to the development team of k6-jslib-aws.

I regularly use k6 in my projects. Now, in the product I am planning to conduct a load test on, there is an AWS S3 Multipart Upload feature. In order to create a load testing scenario for this feature, I have implemented the Multipart Upload functionality in this PR. I would greatly appreciate it if you could review it.

Thank you in advance.

@CLAassistant
Copy link

CLAassistant commented Apr 26, 2023

CLA assistant check
All committers have signed the CLA.

@@ -268,24 +268,180 @@ export class S3Client extends AWSClient {
this._handle_error('DeleteObject', res)
}

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

)
}

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

return new S3Part(partNumber, res.headers['Etag'])
}

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this._handle_error('CompleteMultipartUpload', res)
}

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oleiade oleiade self-assigned this Apr 28, 2023
@oleiade oleiade added the enhancement New feature or request label Apr 28, 2023
@oleiade
Copy link
Member

oleiade commented Apr 28, 2023

こんにちは @ryoshindo、ご協力いただきありがとうございます 🙇🏻

This looks really good, I'll wait for @immavalls to take a look too if she has the time, 👍🏻 on my part, and ready to merge. Next steps on our side will be to put together a documentation PR for the k6 docs website, I'll probably add you as a reviewer there @ryoshindo if that's okay with you, so that you can confirm I documented the feature correctly.

Copy link
Member

@oleiade oleiade left a comment

Choose a reason for hiding this comment

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

👍🏻 🎉

@immavalls
Copy link
Contributor

Thanks for the contribution @ryoshindo 🎉
@oleiade let me know if you need help with the docs, I could work on the doc PR next week probably

@ryoshindo
Copy link
Contributor Author

@oleiade @immavalls ありがとうございます!

Thank you for the positive feedback! I'm glad you find the changes good. Please feel free to add me as a reviewer for the documentation PR on the k6 docs website, I'd be more than glad to help and confirm that the feature is documented correctly. Looking forward to the next steps!

@oleiade oleiade added this to the 0.8.0 milestone May 2, 2023
@oleiade oleiade merged commit bbb0691 into grafana:main May 2, 2023
@immavalls
Copy link
Contributor

immavalls commented May 10, 2023

@ryoshindo, we are already working on the docs. If you have the time for a review, we would appreciate it. Docs in staging: https://mdr-ci.staging.k6.io/docs/refs/pull/1168/merge/javascript-api/jslib/aws/s3client/. It's not allowing me to add you as a reviewer there, not sure why 😢

cc: @oleiade

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants