-
Notifications
You must be signed in to change notification settings - Fork 0
[3.1.4] Service Tags support #19
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
Conversation
a32fdb9 to
5c5b6fe
Compare
thetimpanist
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking of the common use case where a user supplies no tags when creating a database. In that case, DPS will set some default tags for the name of the service so it doesn't show up as empty in the UI. In this case, on the next terraform update, the tags should get saved into terraform state.
Then, if the user doesn't include the optional tags attribute in the service, I want to ensure that the default tags don't get unset here. I didn't see any test case that really covers this use case.
| }, | ||
| "tags": schema.MapAttribute{ | ||
| Optional: true, | ||
| Computed: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this computed flag be on? It doesn't seem like it applies here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from the docs:
For Optional: true, Computed: true attributes, when the user doesn't specify the attribute in their config, the Terraform Framework sets
IsUnknown() = true rather than IsNull() = true. This allows the provider to compute/populate the value from the API response.
5c5b6fe to
e416d85
Compare
| ### Automated Installation (Recommended) | ||
|
|
||
| The Terraform Provider for SkySQL listed on the [Terraform Registry](https://registry.terraform.io/providers/skysqlinc/skysql-beta/). | ||
| The Terraform Provider for SkySQL listed on the [Terraform Registry](https://registry.terraform.io/providers/skysqlinc/beta/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be a wrong link. It has to be .../skysql-beta
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
am having difficulty with local docs generator. will update.
| required_providers { | ||
| skysql = { | ||
| source = "registry.terraform.io/skysqlinc/skysql-beta" | ||
| source = "registry.terraform.io/skysqlinc/beta" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it even work? Are we renaming registry location?
| tags = { | ||
| "name" = var.skysql_service_name | ||
| "environment" = "demo" | ||
| "connectivity" = "private-service-connect" | ||
| "cloud-provider" = "gcp" | ||
| } | ||
| wait_for_creation = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also - Do we support any extra tags? These cannot be added/edited using the UI. So, we would be supporting only via TF? They are not visible on UI anywhere...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DPS supports any number of tags, having name tag being required... Believe this is being added to the UI @ramangoel97
|
closing in favor of https://github.com/skysqlinc/terraform-provider-skysql-beta/pull/21/files |
No description provided.