Autogenerate google_compute_health_check in Terraform #415
Conversation
|
I am a robot that works on MagicModules PRs! I built this PR into one or more PRs on other repositories, and when those are closed, this PR will also be merged and closed. |
rileykarson
force-pushed
the
health-check
branch
from
b7222fa to
19aa7db
Compare
|
I am (still) a robot that works on MagicModules PRs! I just wanted to let you know that your changes (as of commit 3c53717) have been included in your existing downstream PRs. |
|
I am (still) a robot that works on MagicModules PRs! I just wanted to let you know that your changes (as of commit 2d26717) have been included in your existing downstream PRs. |
We've got a bunch of format + if/else logic to handle this particular case: What if I have a method call that needs 3 arguments in one case + just 2 in all other cases? What if adding this argument sometimes causes us to go > 100 characters? What if it's 4 arguments and 2 are optional? Ansible solved this with the `method_decl` and `method_call` functions: Ruby functions that take in a method name and list of arguments and return either a function declaration or a method call. These functions can be given nil arguments that will be ignored and the functions handle all of the formatting issues (what if it's over 100 characters). Formatting function calls is pretty easy for a computer to do. This is mostly a no-op change. There will be a couple of consistent P/C changes, but they're just formatting changes. <!-- Changes per downstream repository. For each repository that you expect to have changed, find the [tag] and write your commit message beneath it. More-specific tags replace less-specific tags. For example, if you provide a message under [all], a message under [puppet], and a message under [puppet-dns], the Terraform repository will have the resulting commit made using the [all] message, the Puppet Compute repository will have its commit made using [puppet], and the Puppet DNS repository will have its commit made using [puppet-dns]. You can delete unused tags, but you don't need to. The structure of the PR body is important to our CI system! The comments can be deleted, but if you want to make the downstream commits sensible, you'll need to leave the dashed line separating this PR's changes from the commit messages for downstream commits. --> ----------------------------------------------------------------- # [all] ## [terraform] ## [puppet] ### [puppet-bigquery] ### [puppet-compute] ### [puppet-container] ### [puppet-dns] ### [puppet-logging] ### [puppet-pubsub] ### [puppet-resourcemanager] ### [puppet-sql] ### [puppet-storage] ## [chef] ### [chef-compute] ### [chef-container] ### [chef-dns] ### [chef-logging] ### [chef-spanner] ### [chef-sql] ### [chef-storage] ## [ansible]
| @@ -0,0 +1,18 @@ | |||
| if _, ok := d.GetOk("http_health_check"); ok { | |||
There was a problem hiding this comment.
Probably needs the licensing code up top - see the encoder for redis_instance.rb
There was a problem hiding this comment.
Also prob not a bad idea to go fmt this file, there's mixed whitespaces (I'm guessing from auto tab-to-spaces?) Sidenote, I kinda want to move towards having these file types to be *.go.erb or similar for mostly IDE purposes/less confusion but I don't expect this to be done lol.
| @@ -32,14 +32,14 @@ | |||
| <% if force_new?(property, object) -%> | |||
| ForceNew: true, | |||
| <% end -%> | |||
| <% unless property.validation.nil? -%> | |||
| <% unless property.validation.nil? || property.output -%> | |||
There was a problem hiding this comment.
I think we still need to have validation if a computed field is "Optional:true" (i.e. allowed to be set from cfg)
There was a problem hiding this comment.
From what I understand, property.output is only Computed and property.default_from_api is a Computed and Optional value so they should still get validation?
There was a problem hiding this comment.
I guess I'm worried that if a property is both marked as output and default_to_api (idk if its possible but I think it is and I could see someone doing this by mistake), this will cause issues.
There was a problem hiding this comment.
Since marking something as both output and default_from_api should be wrong, I don't think it's an issue - a bad config will produce bad results. If it comes up late and we find it should be correct we can make sure it isn't optional with a big ugly conditional, but before then I don't really see the point in guarding against it.
Unfortunately I also can't see an easy place to drop a validation that both fields aren't set like we do for output and required. It looks like there's no protection against setting default_from_api and required together as well.
|
I am (still) a robot that works on MagicModules PRs! I just wanted to let you know that your changes (as of commit ab5d018) have been included in your existing downstream PRs. |
Tracked submodules are build/puppet/_bundle build/puppet/auth build/puppet/bigquery build/puppet/compute build/puppet/sql build/puppet/storage build/puppet/spanner build/puppet/container build/puppet/dns build/puppet/pubsub build/puppet/resourcemanager build/chef/_bundle build/chef/auth build/chef/compute build/chef/sql build/chef/storage build/chef/spanner build/chef/container build/chef/dns build/chef/iam build/terraform build/ansible.
[all]
Minor updates to HealthCheck description, default values.
[terraform]
Autogenerate HealthCheck resource
[puppet]
[puppet-bigquery]
[puppet-compute]
[puppet-container]
[puppet-dns]
[puppet-logging]
[puppet-pubsub]
[puppet-resourcemanager]
[puppet-sql]
[puppet-storage]
[chef]
[chef-compute]
[chef-container]
[chef-dns]
[chef-logging]
[chef-spanner]
[chef-sql]
[chef-storage]
[ansible]