-
Notifications
You must be signed in to change notification settings - Fork 767
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
chore: migrate AWS SDK for JavaScript v2 APIs to v3 #11890
base: sycl
Are you sure you want to change the base?
Conversation
@@ -87,7 +93,7 @@ async function start(param_type, param_label, param_ami, param_spot, param_disk, | |||
const items = ec2disk.split(':'); | |||
params.BlockDeviceMappings = [ {DeviceName: items[0], Ebs: {VolumeSize: items[1]}} ]; | |||
} | |||
const result = await ec2.runInstances(params).promise(); | |||
const result = await ec2.runInstances(params); |
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.
I would also replace this to match v3: https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/client/ec2/command/RunInstancesCommand/
@@ -150,7 +165,7 @@ async function stop(param_label) { | |||
for (const reservation of instances.Reservations) { | |||
for (const instance of reservation.Instances) { | |||
try { | |||
await ec2.terminateInstances({ InstanceIds: [ instance.InstanceId ] }).promise(); | |||
await ec2.terminateInstances({ InstanceIds: [ instance.InstanceId ] }); |
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.
Please update to v3 as well: https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/client/ec2/command/TerminateInstancesCommand/
@@ -138,7 +153,7 @@ async function stop(param_label) { | |||
try { | |||
instances = await ec2.describeInstances({ | |||
Filters: [ { Name: "tag:Label", Values: [ label ] } ] | |||
}).promise(); |
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.
@@ -35,7 +35,13 @@ let reg_token; | |||
|
|||
// starts AWS EC2 instance that will spawn Github runner for a given label | |||
async function start(param_type, param_label, param_ami, param_spot, param_disk, param_timebomb, param_onejob) { | |||
const ec2 = new AWS.EC2(); | |||
const ec2 = new EC2({ |
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.
Please move this code for ec2 object creation outside this function (ideally it should be only called only once before start ort stop function call) since this function can be called in the loop and creating several copies of this EC2 object is not the best practice I guess.
@@ -129,7 +138,13 @@ async function start(param_type, param_label, param_ami, param_spot, param_disk, | |||
async function stop(param_label) { | |||
// last error that will be thrown in case something will break here | |||
let last_error; | |||
const ec2 = new AWS.EC2(); | |||
const ec2 = new EC2({ |
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.
Please try to avoid this code duplication with start function and pass ec2 as start/stop function param.
@@ -1,6 +1,6 @@ | |||
const core = require('@actions/core'); | |||
const github = require('@actions/github'); | |||
const AWS = require('aws-sdk'); | |||
const { EC2, waitUntilInstanceRunning } = require('@aws-sdk/client-ec2'); |
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.
Was you able to test these changes in your own repo or it can potentially break stuff?
@apstasen This PR (and others on public GitHub repos) is meant to be a proof of concept for v2 to v3 migration. The changes are minimal and mostly use codemod, i.e. use v2 style of making API calls and don't add any optimization. Can it be merged after testing, and improvements taken up separately? The API calls have been tested since v3 dev-preview since 2018, and v3 has been GA since Dec 2020. |
AWS SDK for JavaScript v2 will enter maintenance mode on September 8, 2024 and reach end-of-support on September 8, 2025. For more information, check blog post at https://a.co/cUPnyil |
This pull request is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be automatically closed in 30 days. |
AWS SDK for JavaScript v2 has entered maintenance mode. It will reach end-of-support on September 8, 2025. For more information, check blog post at a.co/cUPnyil |
This pull request is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be automatically closed in 30 days. |
From AWS SDK for JavaScript v2 README:
This PR migrates AWS SDK for JavaScript v2 APIs to v3 using aws-sdk-js-codemod.
$ npx [email protected] -t v2-to-v3 devops/actions/**/*.js