-
-
Couldn't load subscription status.
- Fork 297
Feat/migrate aws sdk v3 #698
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
Conversation
📝 WalkthroughWalkthroughThis pull request migrates the project from the legacy AWS SDK to the modular AWS SDK v3. New dependencies were added in the package configuration, and multiple modules have been refactored to use command-based API calls (via the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CF as CloudFormation Client
participant CFService as AWS CloudFormation Service
Client->>CF: send(ListStacksCommand)
CF->>CFService: Request current stacks
CFService-->>CF: Return stack list
alt Stack creation required
Client->>CF: send(CreateStackCommand)
CF->>CFService: Create new stack
CFService-->>CF: Return creation response
Client->>CF: waitUntilStackCreateComplete({client, maxWaitTime})
end
sequenceDiagram
participant Runner as TaskRunner
participant ECS as ECS Client
participant ECSService as AWS ECS Service
participant Kinesis as Kinesis Client
participant KinesisService as AWS Kinesis Service
Runner->>ECS: send(RunTaskCommand)
ECS->>ECSService: Start task execution
ECSService-->>ECS: Return task details
Runner->>ECS: send(DescribeTasksCommand)
ECSService-->>ECS: Return task status
Runner->>Kinesis: send(GetRecordsCommand)
Kinesis->>KinesisService: Fetch log records
KinesisService-->>Kinesis: Return log records
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
package.json (1)
35-43: 💡 Verification agent❓ Verification inconclusive
Consider removing the deprecated aws-sdk v2 package.
While you've successfully added the AWS SDK v3 modules, the old
aws-sdkpackage (v2) is still present at line 43. After completing the migration, you should consider removing this dependency to avoid confusion and reduce your package size.
🏁 Script executed:
#!/bin/bash # Verify if aws-sdk v2 is still being imported anywhere after migration rg -l "require\('aws-sdk'\)" --type ts rg -l "from 'aws-sdk'" --type tsLength of output: 87
Below is a new script to widen the search (across all file types, not just TypeScript) for any residual usage of the deprecated aws-sdk v2:
Once you run the above script, please verify that it produces no output. If no files are listed, it confirms that there are no remaining import statements referencing aws-sdk v2.
Final Review Comment:
Remove unused aws-sdk v2 dependency
While migrating to AWS SDK v3, the old aws-sdk v2 package (listed at line 43 in package.json) appears to be completely unused in the codebase. Our verification (including the extended search across all file types) did not reveal any references to it. To avoid confusion and reduce your package size, please remove the aws-sdk dependency.
🏁 Script executed:
#!/bin/bash # Search for CommonJS and ES Module import patterns for aws-sdk, excluding package.json rg -l -g '!package.json' "require\(['\"]aws-sdk['\"]\)" . rg -l -g '!package.json' "from ['\"]aws-sdk['\"]" .Length of output: 243
Below is a revised review comment that incorporates the extended verification:
Remove Deprecated aws-sdk v2 Dependency
After verifying the codebase using extended searches (both CommonJS and ES module import patterns across all file types excluding package.json), no references to the deprecated aws-sdk v2 were found. This confirms that the migration to AWS SDK v3 is complete and the legacy dependency is unused. Please remove
"aws-sdk": "^2.1081.0"from your package.json (line 43) to avoid confusion and reduce package size.
- Action Item: Remove the aws-sdk dependency from package.json at line 43.
src/model/cloud-runner/providers/aws/services/task-service.ts (1)
59-67:⚠️ Potential issueFix inaccurate “age” calculation.
Using
new Date(Date.now() - creationTime)and then callinggetHours()/getMinutes()returns the local clock time, not total age. Replace it with direct time-difference calculations to accurately track days/hours/minutes since creation.Apply this diff as a sample fix:
- const ageDate: Date = new Date(Date.now() - (element.CreationTime?.getTime() ?? 0)); - CloudRunnerLogger.log( - `Task Stack ${element.StackName} - Age D${Math.floor( - ageDate.getHours() / 24, - )} H${ageDate.getHours()} M${ageDate.getMinutes()}` - ); + const differenceMs = Date.now() - (element.CreationTime?.getTime() ?? 0); + const diffHours = Math.floor(differenceMs / (1000 * 60 * 60)); + const diffMinutes = Math.floor(differenceMs / (1000 * 60)) % 60; + const diffDays = Math.floor(diffHours / 24); + CloudRunnerLogger.log( + `Task Stack ${element.StackName} - Age D${diffDays} H${diffHours % 24} M${diffMinutes}` + );
🧹 Nitpick comments (20)
src/model/cloud-runner/providers/aws/services/garbage-collection-service.ts (1)
71-71: Consider error handling for failed delete operations.While the migration to AWS SDK v3 is correct, you might want to add error handling for the delete operations, as they could fail for various reasons (e.g., insufficient permissions, resource dependencies).
- await cwl.send(new DeleteLogGroupCommand({ logGroupName: element.logGroupName || '' })); + try { + await cwl.send(new DeleteLogGroupCommand({ logGroupName: element.logGroupName || '' })); + } catch (error) { + CloudRunnerLogger.error(`Failed to delete log group ${element.logGroupName}: ${error.message}`); + }src/model/cloud-runner/providers/aws/index.ts (5)
80-80: Consider re-using the CloudFormation client for performance.Re-initializing a new CloudFormation client can be expensive if called frequently. Unless isolation across invocations is needed, re-using a shared client instance might improve efficiency.
94-95: Multiple AWS clients in the same scope.While this approach is valid, you might unify the ECS client and CF client creation in a single place for consistency and maintainability, preventing accidental region mismatches.
97-97: Assigning Kinesis client statically to AwsTaskRunner.Attaching the Kinesis client to
AwsTaskRunneris okay, but consider dependency injection if you want to keep the runner loosely coupled and easier to test.
138-138: Deleting cleanup stack separately.This is consistent with the new approach, but ensure you have logic ensuring the cleanup stack itself is fully inactive before removal to avoid “stack in use” errors.
150-158: Waiting for cleanup stack deletion.Same approach for the cleanup stack. Check whether 200 seconds is long enough, especially in high-load scenarios or large resource dependencies.
src/model/cloud-runner/providers/aws/aws-base-stack.ts (2)
62-62: Describing stacks withDescribeStacksCommand.This asynchronous call is straightforward. Make sure thorough error handling is in place if the stack is absent.
61-61: Use lowercasebooleaninstead ofBoolean.Static analysis flags
Booleanas a banned type. For consistency and clarity, prefer the primitiveboolean.- const stackExists: Boolean = stackNames.includes(baseStackName) || false; + const stackExists: boolean = stackNames.includes(baseStackName) || false;src/model/cloud-runner/providers/aws/aws-job-stack.ts (3)
130-130: Listing stacks without filters.Using
new ListStacksCommand({})enumerates all stacks. This is fine, but filter usage might streamline the logic if needed.
200-200: Creating the cleanup stack.This step ensures leftover resources are removed automatically. Consider logging any potential partial failures.
217-218: Describing base-stack resources.Accessing
this.baseStackNameis consistent. As a small optimization, reuse the result if you already have this data from earlier calls.src/model/cloud-runner/providers/aws/services/task-service.ts (5)
46-48: Consider adding error handling.You’re calling
CF.send(new ListStacksCommand({}))and immediately using its result without atry/catchor fallback. In the event of AWS-related network errors or throttling, the operation may fail.+try { const stacks = (await CF.send(new ListStacksCommand({}))).StackSummaries?.filter( ... ) || []; +} catch (error) { + CloudRunnerLogger.logError(`ListStacksCommand failed: ${error}`); + throw error; +}
98-99: Reuse ECS client if possible.Instantiating
new ECS(...)repeatedly can be costly. Consider reusing a single instance ofECSif you’re calling it often in this file.
109-109: Error handling for DescribeTasks.Like other AWS calls, consider wrapping
ecs.send(new DescribeTasksCommand(...))in a try/catch block to log or handle potential AWS errors gracefully.
159-159: Rename variable to avoid confusion.You’re naming the
CloudWatchLogsclient asecs. Rename it to something likelogsClientorcwlfor clarity.-const ecs = new CloudWatchLogs(); +const logsClient = new CloudWatchLogs(); ... -let logGroupsDescribe = await ecs.send(new DescribeLogGroupsCommand(logStreamInput)); +let logGroupsDescribe = await logsClient.send(new DescribeLogGroupsCommand(logStreamInput));Also applies to: 161-165
193-198: Make S3 calls more robust.
await s3.send(new ListObjectsCommand(listRequest))can fail due to bucket permissions or region mismatch. Consider wrapping in try/catch for reliability.src/model/cloud-runner/providers/aws/aws-task-runner.ts (4)
30-31: Refine static property names if desired.Currently
public static ECSandpublic static Kinesisare uppercase. Consider renaming them toecsClientandkinesisClientfor clarity, or making them private if scoping allows.
78-78: Consider wrapping RunTaskCommand in a try/catch.In the event of an AWS service error, you can log additional info or retry as needed, rather than letting it bubble up unhandled.
190-190: Check for throttling or expired shard iterators.When calling
Kinesis.send(new GetRecordsCommand(...)), the code doesn’t explicitly handle potential throttling or an expired iterator. Consider backoff or error handling for resilience.
229-233: Consider memory usage for uncompressed logs.Decompressing data via
zlib.gunzipSyncand parsing JSON might be large for high-volume logs. If log volumes grow, switch to a streaming / incremental approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mapdist/licenses.txtis excluded by!**/dist/**
📒 Files selected for processing (9)
package.json(2 hunks)src/model/cloud-runner/providers/aws/aws-base-stack.ts(4 hunks)src/model/cloud-runner/providers/aws/aws-error.ts(1 hunks)src/model/cloud-runner/providers/aws/aws-job-stack.ts(7 hunks)src/model/cloud-runner/providers/aws/aws-task-runner.ts(9 hunks)src/model/cloud-runner/providers/aws/cloud-runner-aws-task-def.ts(1 hunks)src/model/cloud-runner/providers/aws/index.ts(4 hunks)src/model/cloud-runner/providers/aws/services/garbage-collection-service.ts(5 hunks)src/model/cloud-runner/providers/aws/services/task-service.ts(7 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
src/model/cloud-runner/providers/aws/services/garbage-collection-service.ts (1)
src/model/cloud-runner/providers/aws/services/task-service.ts (1)
TaskService(30-202)
src/model/cloud-runner/providers/aws/aws-error.ts (1)
src/model/cloud-runner/services/core/cloud-runner-logger.ts (1)
error(24-26)
src/model/cloud-runner/providers/aws/aws-base-stack.ts (1)
src/model/cloud-runner/providers/aws/cloud-formations/base-stack-formation.ts (1)
BaseStackFormation(1-397)
🪛 Biome (1.9.4)
src/model/cloud-runner/providers/aws/aws-base-stack.ts
[error] 60-60: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
🔇 Additional comments (43)
package.json (1)
35-39: Good addition of AWS SDK v3 client packages.The PR correctly adds the necessary AWS SDK v3 modular client packages that are needed for the migration. Each package targets a specific AWS service, which aligns with the modular approach of SDK v3.
src/model/cloud-runner/providers/aws/services/garbage-collection-service.ts (5)
1-8: Good practice using modular imports from AWS SDK v3.You've correctly imported specific clients and commands from the AWS SDK v3 packages, which follows AWS best practices. This modular approach allows for better tree-shaking and smaller bundle sizes compared to importing the entire SDK.
22-24: Client instantiation has been properly updated.The client instantiation has been correctly updated to the AWS SDK v3 pattern, properly passing the region in the constructor.
33-33: Command pattern implementation is correct.You've correctly implemented the AWS SDK v3 command pattern by using the
sendmethod with aStopTaskCommandinstance. This follows AWS's recommended approach for v3.
49-52: Good null check for CreationTime.You've added a null check for
element.CreationTimebefore passing it to theisOlderThan1daymethod, which prevents potential runtime errors. This is a good defensive programming practice.
60-61: TypeScript type safety improvement.Adding the
DeleteStackCommandInputtype fordeleteStackInputimproves type safety and makes the code more maintainable. Good job implementing proper typing as part of the migration.src/model/cloud-runner/providers/aws/aws-error.ts (3)
2-2: Correctly updated AWS SDK import.You've properly updated the import statement to use the modular AWS SDK v3 format, importing only the needed components.
7-7: Parameter type correctly updated.The parameter type for
CFhas been correctly updated fromSDK.CloudFormationtoCloudFormation, reflecting the new import structure.
12-12: Command pattern correctly implemented.You've successfully migrated from the method-based approach with
.promise()to the command pattern usingsend(new DescribeStackEventsCommand()). This follows AWS SDK v3 best practices.src/model/cloud-runner/providers/aws/cloud-runner-aws-task-def.ts (2)
1-1: Updated import to use AWS SDK v3.You've correctly updated the import statement to use the modular AWS SDK v3, importing only the
StackResourcetype that's needed.
6-7:✅ Verification successful
Types properly updated to AWS SDK v3 equivalents.
The property types have been correctly updated from
AWS.CloudFormation.StackResources | undefinedtoArray<StackResource> | undefined, reflecting the type changes in AWS SDK v3.One thing to verify is that all consumers of these properties are aware of the type change from a possibly undefined object to a possibly undefined array.
🏁 Script executed:
#!/bin/bash # Search for any usage of taskDefResources or baseResources properties rg -A 5 "taskDefResources|baseResources" --type tsLength of output: 3625
Verified Type Compatibility for Array Properties
The type update to
Array<StackResource> | undefinedhas been confirmed. Our search verifies that all consumers in files such asaws-task-runner.tsandaws-job-stack.tsinvoke array methods (e.g.,.find()) and correctly account for the possibility of the property being undefined. No further modifications are required.src/model/cloud-runner/providers/aws/index.ts (4)
1-3: Good job migrating to AWS SDK v3 imports.These modular imports are aligned with the PR goal of reducing bundle size and improving clarity. No issues here.
134-134: Method signature update is consistent with AWS SDK v3.Switching from
SDK.CloudFormationtoCloudFormationis aligned with the v3 migration. Ensure all references throughout the codebase are updated accordingly.
136-136: Deleting main stack using command-based API.Calling
CF.send(new DeleteStackCommand(...))directly is correct. Make sure you handle potential errors or race conditions if tasks are still running.
141-149: Waiting for main stack deletion.Using
waitUntilStackDeleteCompletewithmaxWaitTime: 200seconds is helpful. For large or slow stacks, consider verifying if 200 seconds is sufficient.src/model/cloud-runner/providers/aws/aws-base-stack.ts (13)
3-15: Switch to modular CloudFormation imports.Great use of the new v3 command-based API. This makes the code more explicit and aligns with AWS best practices.
25-25: Updating method signature to accept v3 CloudFormation client.This is consistent with the overall migration. Confirm that all callers now pass the v3 client.
31-31: Describing stacks usingDescribeStacksCommandInput.The new typed commands enhance type safety. No concerns here.
34-34: Parameter arrangement.Properly structuring your stack parameters ensures clarity. Good approach in passing them as typed arrays.
39-39: Hash-based versioning.Appending an MD5 hash is a neat way to handle stack updates. This avoids accidental drift.
43-44: Building update stack input.Using typed inputs like
UpdateStackCommandInputand referencing capabilities is aligned with v3’s design.
49-50: Building create stack input.Same pattern. This approach keeps your code consistent and maintainable.
56-58: Listing existing stacks with filters.Your filter logic for listing stacks is correct. The
send+ListStacksCommandapproach is standard for v3.
67-67: CreateStackCommand usage.Clean approach to creating the base stack if it does not exist.
79-85: Waiting for stack creation withwaitUntilStackCreateComplete.This is the recommended approach in v3. Consider verifying if the 200-second wait is adequate for large stacks.
88-88: Log clarity.Good job logging the existing stack version. This helps debug version mismatches.
92-92: Updating stack using new command-based API.Make sure you handle the “no updates are to be performed” scenario gracefully, which you do.
113-118: Waiting for stack update to complete.Same note about verifying the 200-second max wait time. Large updates may exceed that limit.
src/model/cloud-runner/providers/aws/aws-job-stack.ts (7)
1-9: AWS SDK v3 import statements.This modular import pattern enhances maintainability and reduces code bloat.
27-27: Method signature updated to v3 client type.This aligns with the overall migration. Ensure all references are updated across the code.
143-143: DefiningcreateStackInputusing v3 types.This is consistent with the new command-based approach. Keep your parameter usage well-documented.
152-158: Waiting for the job stack to finish creation.
waitUntilStackCreateCompleteis clear. Again, confirm that 200 seconds is enough time for all resources to stabilize.
159-159: Describing the newly created stack.Verifying parameters post-creation is a wise guardrail to detect partial failures.
170-170: Creating cleanup stack input with v3 approach.This is consistent with the job stack creation logic.
210-214: Describing stack resources for the task definition stack.The command-based
DescribeStackResourcesCommandis used correctly. Make sure you handle cases where resources might be in transitional states.src/model/cloud-runner/providers/aws/services/task-service.ts (5)
1-23: Migration to AWS SDK v3 imports looks good.These imports correctly reference the new modular packages from AWS SDK v3.
42-42: Generic type usage is correct.Defining
const result: StackSummary[] = []ensures typed clarity for stack summaries.
55-57: Validate handling of undefined CreationTime.Currently, if
CreationTimeis undefined, the code logs a message but continues using0as a fallback. This may skew the “age” calculation.Would you like to skip these stacks altogether or handle them differently? Confirm the intended behavior when
CreationTimeis missing.Also applies to: 59-60
94-94: LGTM on typed task result.Defining
resultas{ taskElement: Task; element: string }[]is clear and consistent.
122-122: Confirm logic for skipping tasks with no creation date.If
createdAtis missing, the code simply logs and continues. Verify that this behavior aligns with your requirements and doesn’t hide potential data issues.src/model/cloud-runner/providers/aws/aws-task-runner.ts (3)
1-16: AWS SDK v3 imports look correct.These updated imports for ECS and Kinesis align with the new command-based approach in v3.
126-132: Good usage of waiters.Using
waitUntilTasksRunningis a clean approach to block until ECS tasks are running. Thetry/catchwithcore.setFailed(error)is an appropriate errorflow.
257-263: Confirm shard iterator type.Using
'TRIM_HORIZON'retrieves all historical records from the earliest point. Verify that you actually need from the beginning, rather than'LATEST'to get only new logs.
| if (!element.CreationTime) { | ||
| CloudRunnerLogger.log(`${element.StackName} due to undefined CreationTime`); | ||
| } |
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.
Apply the same age calculation fix here as well.
For the base stacks, you have the same pattern for computing “age” using new Date(...). Apply the same fix for logical consistency.
Also applies to: 80-81
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/model/cloud-runner/providers/aws/services/task-service.ts (2)
135-163: Consider adding try/catch blocks to other AWS SDK calls.You've added a try/catch block to the
awsDescribeJobmethod, which is great. For consistency and robustness, consider adding similar error handling to other methods that make AWS SDK calls, likegetCloudFormationJobStacks,getTasks,getLogGroups, andgetLocks.For example:
public static async getCloudFormationJobStacks() { const result: StackSummary[] = []; CloudRunnerLogger.log(``); CloudRunnerLogger.log(`List Cloud Formation Stacks`); process.env.AWS_REGION = Input.region; const CF = new CloudFormation({ region: Input.region }); + try { const stacks = (await CF.send(new ListStacksCommand({}))).StackSummaries?.filter( (_x) => _x.StackStatus !== 'DELETE_COMPLETE' && _x.TemplateDescription !== BaseStackFormation.baseStackDecription, ) || []; // rest of the method... return result; + } catch (error) { + CloudRunnerLogger.error( + `Failed to get CloudFormation job stacks: ${error instanceof Error ? error.message : String(error)}`, + ); + throw error; + } }
186-187: Optimize age calculation in getLogGroups method.The age calculation in the getLogGroups method has the same issue as in other methods.
-const ageDate: Date = new Date(Date.now() - element.creationTime); +const ageInMs: number = Date.now() - element.creationTime; +const days = Math.floor(ageInMs / (1000 * 60 * 60 * 24)); +const hours = Math.floor((ageInMs % (1000 * 60 * 60 * 24)) / (1000 * 60 * 60)); +const minutes = Math.floor((ageInMs % (1000 * 60 * 60)) / (1000 * 60));Then update the logging in lines 188-192 to use these variables directly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (1)
src/model/cloud-runner/providers/aws/services/task-service.ts(6 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/model/cloud-runner/providers/aws/services/task-service.ts (2)
src/model/cloud-runner/providers/aws/cloud-formations/base-stack-formation.ts (1)
BaseStackFormation(1-397)src/model/cloud-runner/services/core/cloud-runner-logger.ts (1)
error(24-26)
🔇 Additional comments (9)
src/model/cloud-runner/providers/aws/services/task-service.ts (9)
1-23: Nice job on modularizing the AWS SDK imports.The change from a single
aws-sdkimport to modular imports from specific AWS SDK v3 packages is a great improvement. This approach results in smaller bundle sizes by only importing what's needed.
42-48: Good transition to command pattern.The switch from promise-based calls to the command pattern is well implemented. The type change from
StackSummariestoStackSummary[]also properly matches the v3 SDK return type.
55-60: Check for undefined CreationTime is a good addition, but age calculation could be improved.The null check for
CreationTimeand the use of optional chaining with nullish coalescing is a great defensive programming practice. However, the age calculation method is not the most accurate way to calculate days.-const ageDate: Date = new Date(Date.now() - (element.CreationTime?.getTime() ?? 0)); +const ageInMs: number = Date.now() - (element.CreationTime?.getTime() ?? 0); +const days = Math.floor(ageInMs / (1000 * 60 * 60 * 24)); +const hours = Math.floor((ageInMs % (1000 * 60 * 60 * 24)) / (1000 * 60 * 60)); +const minutes = Math.floor((ageInMs % (1000 * 60 * 60)) / (1000 * 60));Then update the logging in lines 62-65 to use these variables directly.
76-80: Same age calculation issue for base stacks.The same issue with age calculation exists here for base stacks.
-const ageDate: Date = new Date(Date.now() - (element.CreationTime?.getTime() ?? 0)); +const ageInMs: number = Date.now() - (element.CreationTime?.getTime() ?? 0); +const days = Math.floor(ageInMs / (1000 * 60 * 60 * 24)); +const hours = Math.floor((ageInMs % (1000 * 60 * 60 * 24)) / (1000 * 60 * 60)); +const minutes = Math.floor((ageInMs % (1000 * 60 * 60)) / (1000 * 60));
93-109: Good implementation of getTasks method with AWS SDK v3.The type changes and command pattern implementation are well done. The method correctly handles the new AWS SDK v3 structure and maintains the original functionality.
133-164: Excellent error handling improvement in awsDescribeJob.The addition of a try/catch block with specific error handling is a significant improvement. The error message includes the error type check to ensure the message is properly extracted, and the job ID is included for better debugging.
144-148: Same age calculation issue in awsDescribeJob.The age calculation has the same issue as noted earlier.
-const ageDate: Date = new Date(Date.now() - (stack.CreationTime?.getTime() ?? 0)); +const ageInMs: number = Date.now() - (stack.CreationTime?.getTime() ?? 0); +const days = Math.floor(ageInMs / (1000 * 60 * 60 * 24)); +const hours = Math.floor((ageInMs % (1000 * 60 * 60 * 24)) / (1000 * 60 * 60)); +const minutes = Math.floor((ageInMs % (1000 * 60 * 60)) / (1000 * 60));Then update the message in lines 148-154 to use these variables directly.
165-177: Well-structured update for getLogGroups method.The changes to the getLogGroups method are well-implemented. The type change from
LogGroupstoArray<LogGroup>and the introduction of the command pattern are appropriate for the AWS SDK v3 migration.
198-205: Nicely implemented getLocks method update.The S3 client instantiation and command pattern conversion are well done.
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.
Looks good to me
src/model/cloud-runner/providers/aws/cloud-runner-aws-task-def.ts
Outdated
Show resolved
Hide resolved
|
If someone can verify that this works, we should be good to merge it. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #698 +/- ##
==========================================
+ Coverage 37.04% 37.12% +0.07%
==========================================
Files 77 77
Lines 3131 3157 +26
Branches 647 623 -24
==========================================
+ Hits 1160 1172 +12
- Misses 1828 1985 +157
+ Partials 143 0 -143
🚀 New features to boost your workflow:
|
|
Amazing, looks great! |
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.
Very pleased to have this improvement. Thanks. The testing steps you mention are also helpful. Lgtm.
|
Released in v4.4.0 |

Pull Request: Migrate AWS SDK from v2 to v3
I've avoided affecting existing code and functionality as much as possible by keeping the changes to aws-sdk v3 only.
Changes
aws-sdkdependency with modular SDK v3 packagesclient.send(new Command()))waitUntilfunctionsFiles Changed
package.json: Updated dependenciessrc/model/cloud-runner/providers/aws/**/*.ts: Updated AWS SDK imports and usage patternsTesting
Verified with AWS integration tests that functionality remains intact.
I ran just one job with Webgl with via Build Profile ubuntu, to reduce action minutes:
https://github.com/MtMath/unity-builder/actions/runs/14244237443/job/39921140042
Considerations
Summary by CodeRabbit
These updates help ensure more efficient and stable cloud interactions, contributing to a smoother overall experience.