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

SQL DB Instance has attribute first_ip_address #1050

Merged
merged 4 commits into from
Mar 2, 2018

Conversation

genevieve
Copy link

@genevieve genevieve commented Feb 5, 2018

Genevieve LEsperance and others added 4 commits February 5, 2018 10:19
@genevieve
Copy link
Author

Hey @danawillow. I opened this PR after discussions in #912 and hashicorp/terraform#17048.

Is there anything about this attribute or it's implementation that we can fix? This is currently a blocker for us as there is no way to have this particular resource have a count.

@danawillow
Copy link
Contributor

I see in the other issue that a workaround of using TF_WARN_OUTPUT_ERRORS=1 until the new syntax comes out was suggested- did that work?

@genevieve
Copy link
Author

genevieve commented Mar 1, 2018

Since the terraform doesn't apply without that environment variable, it means resources that all our teams use in CI/CD pipelines would have to support that, as well as asking it of all the users that go directly to use these terraform templates. It wasn't really something we wanted to recommend since it risks actually swallowing relevant errors.

When we choose one output syntax over the other either the database-turned-off case works and the database-turned-on fails during terraform apply, and choosing a different syntax causes the opposite.

Ideally we could have a terraform module for creating this database, but due to the inability to actually make it work with the terraform syntax and the resource's structure, it has to be mandatory. Currently, teams that weren't opting in to this database are seeing a recurring issue opened here (#1138) and they can't avoid it by opting out of the database since we had to make it a mandatory resource.

@danawillow
Copy link
Contributor

Thanks @genevieve! That sounds reasonable, and the code looks good. Merging now.

@danawillow danawillow merged commit 7e358d8 into hashicorp:master Mar 2, 2018
@danawillow
Copy link
Contributor

Oh wait, I got ahead of myself. I'm going to keep it in, but would you mind sending a follow-up PR for changes to the docs to add this attribute? Please tag me in it and I'll review it right away.

modular-magician added a commit to modular-magician/terraform-provider-google that referenced this pull request Sep 27, 2019
@ghost
Copy link

ghost commented Mar 29, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants