Skip to content
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

allow users know when to enable and disable performance insights #169

Merged
merged 6 commits into from
Apr 8, 2024

Conversation

FolarinOyenuga
Copy link
Contributor

No description provided.

variables.tf Outdated
@@ -97,7 +97,7 @@ variable "ca_cert_identifier" {

variable "performance_insights_enabled" {
type = bool
description = "Enable performance insights for RDS?"
description = "Enable performance insights for RDS? This is disabled by default. However a user can enable it if certain insights are needed from the Database. However, the user should ensure it is disabled once the desired outcome is achieved."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe remove the second however and change to "The user should also ensure..."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Sablu.

variables.tf Outdated
@@ -97,7 +97,7 @@ variable "ca_cert_identifier" {

variable "performance_insights_enabled" {
type = bool
description = "Enable performance insights for RDS?"
description = "Enable performance insights for RDS? This is disabled by default. However a user can enable it if certain insights are needed from the Database. However, the user should ensure it is disabled once the desired outcome is achieved."
Copy link
Contributor

@jaskaransarkaria jaskaransarkaria Apr 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Enable performance insights for RDS? Note: the user should ensure insights are disabled once the desired outcome is achieved."

I don't think you need to explicitly mention the other things as that's implicit from reading the code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Jazz. Updated it.

@FolarinOyenuga FolarinOyenuga merged commit f4b80c4 into main Apr 8, 2024
@FolarinOyenuga FolarinOyenuga deleted the perf-insights-update branch April 8, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants