Add Cloud Composer Environments resource #2001
Conversation
emilymye
changed the title
|
Basic Test Run: Update Test Run: TODO |
emilymye
force-pushed
the
cloud_composer
branch
from
03e58ea to
129c6dc
Compare
There was a problem hiding this comment.
Thanks for dealing with this! It sucks that we can't use MM as-is, and we probably could add some functionality into it that would help, but I agree that right now probably the best thing is to get something working, and then we can backport it later if we can.
| }, | ||
|
|
||
| Timeouts: &schema.ResourceTimeout{ | ||
| Create: schema.DefaultTimeout(3600 * time.Second), |
There was a problem hiding this comment.
Mind adding a quick comment here that this is indeed the default timeout because composer takes so long and not because this is a typo?
| Optional: true, | ||
| Elem: &schema.Schema{Type: schema.TypeString}, | ||
| }, | ||
| "state": { |
There was a problem hiding this comment.
I could be wrong, but these last four probably aren't necessary to include, unless you think people will want to check their values with terraform show or use them for interpolation
| return handleNotFoundError(err, d, fmt.Sprintf("ComposerEnvironment %q", d.Id())) | ||
| } | ||
|
|
||
| // Set from d.GetProject |
There was a problem hiding this comment.
nit: d.GetProject isn't a thing, probably just getProject
| } | ||
|
|
||
| if d.HasChange("config.0.software_config.0.env_variables") { | ||
| patchObj := &composer.Environment{ |
There was a problem hiding this comment.
Just curious- what happens if we specify a patch object with all the fields set via the same expander we use in create, and then here we only worry about setting the correct mask? would that work or would it complain that we set fields that weren't in the mask?
There was a problem hiding this comment.
TBH I'm not sure - I am using expandComposerEnvironmentConfig above so there is a little reuse? I didn't want to deal with the case that they might not enjoy a particular field being included somewhere, so I thought it would be overall less to worry about if I used the expander and then created "immutable" throwaway object with only the fields being updated, in case.
|
|
||
| // Checks that all updatable fields can be updated in one apply | ||
| // (PATCH for Environments only is per-field) and that reverting | ||
| // config force-updates back to default. |
There was a problem hiding this comment.
Which is the part of the test that reverts config?
There was a problem hiding this comment.
I removed it after I realized we don't force-update back to default unless explicitly set to, will update comment
| (Optional) | ||
| The set of Google API scopes to be made available on all node | ||
| VMs. Cannot be updated. If empty, defaults to | ||
| ["https://www.googleapis.com/auth/cloud-platform"] |
There was a problem hiding this comment.
can you put this in `s so it doesn't show up as a link?
|
|
||
| * `env_variables` - | ||
| (Optional) | ||
| Additional environment variables to provide to the Apache Airflow scheduler, worker, and webserver processes. Environment variable names must match the regular expression [a-zA-Z_][a-zA-Z0-9_]*. They cannot specify Apache Airflow software configuration overrides (they cannot match the regular expression AIRFLOW__[A-Z0-9_]+__[A-Z0-9_]+), and they cannot match any of the following reserved names: |
There was a problem hiding this comment.
can you put regexes in `s? GH is previewing things funny because of underscores :(
| This resource provides the following | ||
| [Timeouts](/docs/configuration/resources.html#timeouts) configuration options: | ||
|
|
||
| - `create` - Default is 6 minutes. |
There was a problem hiding this comment.
I think these need to be updated
| Composer service may only detect config errors several (e.g. ~40-50 minutes) into | ||
| the creation process and may only have limited error reporting. If you encounter confusing | ||
| or uninformative errors, please verify your config, especially Airflow properties in `software_config` or | ||
| [shared VPC config](#Configuring-Kubernetes-Engine-and-Shared-VPC) in `node_config`. |
There was a problem hiding this comment.
I think you need a full link here
There was a problem hiding this comment.
I just removed this part so nvm
emilymye
force-pushed
the
cloud_composer
branch
from
43c2e93 to
f9bf24d
Compare
|
Up-to-date test runs as of Monday EOD: HUZZAH |
emilymye
force-pushed
the
cloud_composer
branch
from
78f410e to
d1660e7
Compare
|
@emilymye by any chance you have example terraform config for us to follow? thanks! |
Adds Cloud Composer Environment resource and test sweeper
|
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
locked and limited conversation to collaborators
Fixes #1588
Deleted initial code from when I thought I would use MM to do this. We aren't because: