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

Optionally Enable "redis.googleapis.com" Service #3

Conversation

trotttrotttrott
Copy link
Contributor

Enables the "redis.googleapis.com" service on the targeted project if the enable_apis variable is true. Defaults to true with what we consider to be the most basic module use case in mind - a brand new project is targeted and service enablement is not already managed by other means.

Integration tests now pass successfully when targeting a new, pristine project. As previously or with enable_apis set to false, tests will fail run against a pristine project with Error 403: Google Cloud Memorystore for Redis API has not been used in project <PROJECT_ID> before or it is disabled....

Enables the "redis.googleapis.com" service on the targeted project if
the `enable_apis` variable is true.

Defaults to `true` with what we consider to be the most basic module
usecase in mind - a brand new project is targeted and service
enablement is not already managed by other means.
@morgante morgante requested a review from adrienthebo January 15, 2019 23:11
Copy link

@adrienthebo adrienthebo left a comment

Choose a reason for hiding this comment

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

I've a handful of small comments, nothing really substantive. Let's get those small fixups in and we can merge. 👍

variables.tf Outdated

variable "enable_apis" {
description = "Enable required APIs for Cloud Memorystore."
default = true

Choose a reason for hiding this comment

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

Boolean variable defaults should be quoted because terraform will coerce true to 1, and this behavior will change on 0.12.

main.tf Outdated
}

resource "google_project_service" "redis" {

Choose a reason for hiding this comment

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

Heads up - terraform fmt will remove this space, so we might as well remove it in the PR.

@morgante
Copy link
Contributor

@trotttrotttrott Please run make generate_docs and terraform fmt.

Terraform docs recommend use of strings for boolean values to avoid caveats in
the conversion process.
@trotttrotttrott
Copy link
Contributor Author

@adrienthebo I've addressed your comments. Please take another look when you have a chance!

Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

LGTM

@morgante morgante merged commit 1fd6cae into terraform-google-modules:master Jan 16, 2019
pkatsovich pushed a commit to recurly/terraform-google-memorystore that referenced this pull request Aug 2, 2020
…odules/ludo-simple-example-and-tests

Fix simple example and tests
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