-
Notifications
You must be signed in to change notification settings - Fork 0
Add support for Deployment Monitoring #6
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
Add GitHub Actions workflow to test ECS Express service deployment. Changes: - Copy test-action.yml from feature/express-service-deployment - Update to use service-name parameter (required) - Configure to trigger on feature/integration-tests branch - Test deployment with public nginx image - Display service ARN, endpoint, and status outputs
Remove status output line as it's not currently provided by the action.
For testing, use the source index.js file directly instead of requiring a built dist folder.
- Build the action using ncc to create bundled dist/index.js - Remove dist/ from .gitignore so it can be committed - Revert action.yml to use dist/index.js as entry point GitHub Actions require the dist folder to be committed.
… timeout to 15 minutes
…mentArn correctly
| // Check if service is ACTIVE | ||
| if (statusCode === 'ACTIVE') { | ||
| if (!serviceActive) { | ||
| serviceActive = true; |
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 might have missed this, where do we use this serviceActive, or do we plan to use it in follow up CRs??
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.
My plan is to potentially add something around the catch block that mentions the service isn't active in edge cases
|
|
||
| // Create or update the service | ||
| let response; | ||
| let deploymentStartTime; |
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.
this can be just let deploymentStartTime = new Date() ?
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.
Yes, it could be
|
|
||
| try { | ||
| // Step 1: Check service status using DescribeExpressGatewayService | ||
| const describeServiceCommand = new DescribeExpressGatewayServiceCommand({ |
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.
we dont need to call describeGWService every time, right? This can happen before the while(true)?
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.
Good point, fixing in the next one
In this PR: