-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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(ecs-patterns): healthCheck
property for ApplicationLoadBalancedFargateService
#28797
feat(ecs-patterns): healthCheck
property for ApplicationLoadBalancedFargateService
#28797
Conversation
…lancedFargateServiceProps
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.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request
. Additionally, if clarification is needed add Clarification Request
to a comment.
healthCheck
property to ApplicationLoadBalancedFargateService
healthCheck
property to ApplicationLoadBalancedFargateService
I think I stated the reasons for not including these in the PR description, if someone from the AWS team can confirm that this is okay. If not I can push the updates to this PR, or one of the maintainers can |
Hi @Nikola-Milovic, thank you for your contribution! For every new feature we require integration tests, README changes and unit tests. You can find other |
@aaythapa Thanks for the links! I managed to navigate my way around the repo and figure out the tests. EDIT: Seems that the current test won't deploy properly and is stuck in redeploy loop, will look into it and fix it! |
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
healthCheck
property to ApplicationLoadBalancedFargateService
healthCheck
property to ApplicationLoadBalancedFargateService
healthCheck
property to ApplicationLoadBalancedFargateService
healthCheck
property for ApplicationLoadBalancedFargateService
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.
💪
Pull request has been modified.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…edFargateService` (#28797) Closes #28796 Like with other `aws-ecs-pattern` high level constructs, some properties aren't exposed. For `aws_ecs_patterns`.`ApplicationLoadBalancedFargateService` this is `healthCheck` property of the container. There are already similar PRs #18219 for other constructs in the `aws_ecs_patterns` lib *Note*: I didn't add any test cases since `ApplicationLoadBalancedFargateService` does not seem to have them like the others do. I can maybe come back with another PR to add the test cases. I didn't update the docs since it's a minor addition. And there is already an example for the `QueueProcessingFargateService` that uses the `healthCheck`, it might be redundant to add the same code example with different constructor name. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Closes #28796
Like with other
aws-ecs-pattern
high level constructs, some properties aren't exposed. Foraws_ecs_patterns
.ApplicationLoadBalancedFargateService
this ishealthCheck
property of the container.There are already similar PRs #18219 for other constructs in the
aws_ecs_patterns
libNote:
I didn't add any test cases since
ApplicationLoadBalancedFargateService
does not seem to have them like the others do. I can maybe come back with another PR to add the test cases.I didn't update the docs since it's a minor addition. And there is already an example for the
QueueProcessingFargateService
that uses thehealthCheck
, it might be redundant to add the same code example with different constructor name.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license