Feat cloud cost credentials data - 218 #317
Conversation
env0/data_aws_cost_credentials.go
Outdated
| d.SetId(credentials.Id) | ||
| d.Set("name", credentials.Name) |
There was a problem hiding this comment.
Please use the new common writeResourceData instead. Have a look at existing usages of it to get the idea.
Note: this comment is relevant to all other resources
There was a problem hiding this comment.
Good point ill do it, just a little note, the function you mention based on the name of the field in schema against the field name in the struct , in aws creds we have 'arn' in schema but the struct field is 'RoleArn so in some places like that we cant use those functions, or we can just change the name of the field in struct/schema
There was a problem hiding this comment.
So how did you solve that arn -> RoleArn tranform?
There was a problem hiding this comment.
In this specific code there is no arn attribute so I do used the function you suggested , but in resource(no data resource) for example I cant use it, another problem in this function is when the value of the filed in the struct is struct itself like value in awscredentialscreate but for this I have a solution, anyway its not enough because the problem we talked above, unless we change the schema field name to role_arn
There was a problem hiding this comment.
Ok understood.
Regarding your last sentence - I do think we should change it to role_arn so:
- api and schema are aligned
- We can use this util
env0/data_google_cost_credentials.go
Outdated
| } | ||
|
|
||
| if len(credentialsByNameAndType) > 1 { | ||
| return client.Credentials{}, diag.Errorf("Found multiple Google cost Credentials for name: %s", name) |
There was a problem hiding this comment.
Please align this error message with all other error messages in this file...
…into feat_cloud_cost_cred_data-#218
Co-authored-by: Yaron Yarimi <yaronya@users.noreply.github.com>
…into feat_cloud_cost_cred_data-#218
|
@samuel-br let's put this PR on hold until we finish with #307 as I see similar typos here and there Also, did you see
|
…into feat_cloud_cost_cred_data-#218
I fix it as we did in #307 with little help from function closers, also fix typo I found |
github-actions
bot
added
ready to merge
Issue & Steps to Reproduce / Feature Request
#218
Solution
**part of solution for #218, along with #252 **