fix: fixes the 'type' property of 'user' varaible #226
Conversation
|
/run pipeline |
variables.tf
Outdated
| password = string # pragma: allowlist secret | ||
| type = string # "type" is required to generate the connection string for the outputs. | ||
| password = string # pragma: allowlist secret | ||
| type = optional(string) # "type" is required to generate the connection string for the outputs. |
There was a problem hiding this comment.
hmm, we are now contradicting what that code comment says: "type" is required to generate the connection string for the outputs.
@Khuzaima05 @imprateeksh @shemau if type is made optional, do we break the outputs if its not passed?
There was a problem hiding this comment.
The provider should default the value to database, so this should probably never have been required. If there was a dependency (to make it required) with users this was likely removed by the work to make it use the administrator account automatically created during the database creation.
So I am pretty certain this is not going to cause issues. This should probably be tested/validated at least manually before merging.
The comment should be removed and the change should be made to all 8 databases if it is made to 1.
There was a problem hiding this comment.
@Khuzaima05 can you do some testing and report findings? If we are good remove the comment.
And as Steve said, we will want to do this to all ICD modules
There was a problem hiding this comment.
@ocofaigh , When i try to test for optional value for type.
I have set value for var.users in complete example as :
users = [ {
name = "Khuzaima"
password = "Khuzaima8953153811"
} ]
I was getting this error
elasticsearch_cluster_settings.global: Creating...
elasticsearch_index.test: Creating...
╷
│ Error: Head "https://fbed8076-2806-4c7c-b31a-df85f0606ac7.c13paqsd05a0ept695ng.databases.appdomain.cloud:30750": context deadline exceeded
│
│ with elasticsearch_index.test,
│ on main.tf line 75, in resource "elasticsearch_index" "test":
│ 75: resource "elasticsearch_index" "test" {
│
╵
╷
│ Error: Head "https://fbed8076-2806-4c7c-b31a-df85f0606ac7.c13paqsd05a0ept695ng.databases.appdomain.cloud:30750": context deadline exceeded
│
│ with elasticsearch_cluster_settings.global,
│ on main.tf line 83, in resource "elasticsearch_cluster_settings" "global":
│ 83: resource "elasticsearch_cluster_settings" "global" {
Then I try to run complete example in main branch without any change.
There also i am getting same error during terraform apply:
time_sleep.wait: Still creating... [10s elapsed]
time_sleep.wait: Creation complete after 15s [id=2024-07-18T06:53:23Z]
elasticsearch_cluster_settings.global: Creating...
elasticsearch_index.test: Creating...
╷
│ Error: Head "https://849fdfc8-4eb2-4da0-b0e5-fcb5f56813f4.c5km1ted03t0e8geevf0.databases.appdomain.cloud:31300": context deadline exceeded
│
│ with elasticsearch_index.test,
│ on main.tf line 75, in resource "elasticsearch_index" "test":
│ 75: resource "elasticsearch_index" "test" {
│
╵
╷
│ Error: Head "https://849fdfc8-4eb2-4da0-b0e5-fcb5f56813f4.c5km1ted03t0e8geevf0.databases.appdomain.cloud:31300": context deadline exceeded
│
│ with elasticsearch_cluster_settings.global,
│ on main.tf line 83, in resource "elasticsearch_cluster_settings" "global":
│ 83: resource "elasticsearch_cluster_settings" "global" {
│
╵
|
While working on this PR, I am encountering this error while running complete example: Its a generic error i am getting irrespective of this issue |
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
|
🎉 This issue has been resolved in version 1.15.2 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Description
Fixd the users variable to make type property optional as per documentation
issue: #219
Release required?
x.x.X)x.X.x)X.x.x)Release notes content
Run the pipeline
If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.
Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:
Checklist for reviewers
For mergers