-
Notifications
You must be signed in to change notification settings - Fork 4k
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(route53): RecordSet
routing policy properties, HeathCheck
#4413
Conversation
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
RecordSet
propertiesRecordSet
routing policy properties, HeathCheck
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
It might be interesting to add static methods (one per routing policy type) to build multiple records for the same public static generateFailoverRecords(primary: RecordSetOptions, secondary: RecordSetOptions)
public static generateWeightedRecords(...weightedRecords: RecordSetOptions & {weight: number})
// ... |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
const healthCheck = new CfnHealthCheck(this, 'Resource', { healthCheckConfig: props }); | ||
this.healthCheckId = healthCheck.ref; | ||
} | ||
} |
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.
awslint really doesn't like what I'm doing there. What would be the expected pattern?
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.
what did it complain about?
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
hey @nmussy - will start taking a look at this one tomorrow. Any high level feedback you'd like specifically while you work through your TODO list? It might also help to separate the routing policy change (which addresses the issue referenced) and the HealthCheck constructs in separate PRs. but up to you! |
Hey @shivlaks, That "todo list" should really be a "help me list" 😄. The biggest design decision is whether or not to add higher level L2 methods (which I think we should). As for splitting the PR, I agree, it feels a little unnecessary to throw |
any idea when this PR will be merged? |
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
5 similar comments
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
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.
still have todo list to work through for this PR, I can start taking a look later this week.
@@ -451,3 +594,54 @@ export class ZoneDelegationRecord extends RecordSet { | |||
}); | |||
} | |||
} | |||
|
|||
/** | |||
* Contient code |
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.
minor: typo on Continent
const healthCheck = new CfnHealthCheck(this, 'Resource', { healthCheckConfig: props }); | ||
this.healthCheckId = healthCheck.ref; | ||
} | ||
} |
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.
what did it complain about?
closing this PR as it's stale to reflect that this task is available to pick up on the issue. PR can be reopened or a new one can be published in it's place. |
RecordSet
routing policy propertiesHealthCheck
constructsEndpointHealthCheck
for IP and DNS health checksAlarmHealthCheck
for CloudWatch alarm health checksCalculatedHealthCheck
for health checks checking the health of other health checksaws-ec2
TODO:
awslint
errors due to baseHealthCheck
classHealthCheck
tag generationregion: Aws.REGION
is sufficient forAlarmHealthCheck
CloudWatch alarmsFixes #4391
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license