chore(rds): improve error messages for providing a vpc to cluster

[prasanthlouis]

Just an idea/I was curious.

Personally, when I stumbled upon this error I got a bit confused because I wasn't sure what instanceProps was. I think the message 'Provide either vpc or instanceProps.vpc, but not both" provides the same explanation for both cases?

Or we rephrase the error message to not use "If instanceProps was provided", because the code can figure whether instanceProps was provided with another if condition Then the message would just be "VPC must be provided". Hence, someone who is unaware of instanceProps, doesn't need to be notified of it. Just an idea though and it's nitpicking! Thoughts?

Reason for this change

Better error message when running into this error

Description of changes

Consolidated logs statements

Description of how you validated changes

I have not validated anything because this was just an idea/I was curious

Checklist

• [X ] My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

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.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[scanlonp]

Hey @prasanthlouis, I think you should write the error messages in a way that is the most clear to you. If the current message did not make sense, of course feel free to improve it!

I do not have too much experience using rds clusters, but I could imagine some helpful errors to be:

A VPC must be provided, either through vpc or instanceProps.vpc
Only one of vpc and instanceProps.vpc can be provided

If you think consolidating is best, let's do that.

However, you will need to edit the unit tests in aws-rds/test/cluster.test.ts to use your new error messages. That is why your build is failing.

[prasanthlouis]

Hey @prasanthlouis, I think you should write the error messages in a way that is the most clear to you. If the current message did not make sense, of course feel free to improve it!

I do not have too much experience using rds clusters, but I could imagine some helpful errors to be:

A VPC must be provided, either through vpc or instanceProps.vpc Only one of vpc and instanceProps.vpc can be provided

If you think consolidating is best, let's do that.

However, you will need to edit the unit tests in aws-rds/test/cluster.test.ts to use your new error messages. That is why your build is failing.

Thanks for the reply @scanlonp! I like the consolidation message because there's another similar message regarding VPC subnets right underneath this line as well!

Thanks for informing me of the failing test. I've updated that as well! Let me know if there's anything else I can do to assist. Cheers!

[mergify]

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).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: bea4d2f
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

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).

[aws-cdk-automation]

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.