Closed ca-nguyen closed 3 years ago
go for it - please be sure to update the PR description to include
runJob
andcall
APIs that were added in the most recent commit.
Done! Thank you!
LGTM besides some minor updates to the docs.
Thanks for reviewing! I applied your suggested changes
I see there are unit tests, but did you do any manual testing to ensure each step can be created with Step Functions through the SDK?
I did not do any testing outside of unit tests. I'll make sure to manual test before merging
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository
I see there are unit tests, but did you do any manual testing to ensure each step can be created with Step Functions through the SDK?
Tested manually and uncovered a few bugs:
LogOptions
param from eks::runJob example as it does not allow to retrieve of logs (only possible with eks::runJob.sync)
Made the changes in the last commitsI see there are unit tests, but did you do any manual testing to ensure each step can be created with Step Functions through the SDK?
Tested manually and uncovered a few bugs:
- eks::call does not have option to run a job (.sync)
- removed
LogOptions
param from eks::runJob example as it does not allow to retrieve of logs (only possible with eks::runJob.sync) Made the changes in the last commits
glad we caught those. be sure to expand on testing methodology and the steps we followed.
not for this PR:
thought: how can we automate that testing (via unit/simple integ tests). It will be helpful to establish so that we responsibly introduce service integrations. More importantly, regressions are easy to introduce if we don't have automated tests in place.
Issue #, if available: #106
Description of changes: Added:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.