Feature/groups_datasource show_all_flag #520
Conversation
|
@manicminer I have fixed the merge conflicts - Apologies for the delay. |
manicminer
force-pushed
the
feature/groups_datasource-show-all-flag
branch
from
6afa9ab to
944a7a6
Compare
There was a problem hiding this comment.
Thanks again for this @Threpio! I pushed some changes but I noted them inline below in case it's useful to you.
Test results
| if err != nil { | ||
| return tf.ErrorDiagF(err, "Could not retrieve groups") | ||
| } |
There was a problem hiding this comment.
We try to check errors separately so it's more obvious when they occur
| if result == nil { | ||
| return tf.ErrorDiagF(errors.New("API returned nil result"), "Bad API Response") | ||
| } |
There was a problem hiding this comment.
We should check for nil pointers before dereferencing as doing *result when result is nil would cause a crash
| return tf.ErrorDiagPathF(err, "return_all", "No groups found") | ||
| } | ||
|
|
||
| groups = append(groups, *result...) |
There was a problem hiding this comment.
This notation is a useful shortcut to iterating with a for loop
There was a problem hiding this comment.
Didn't know this one - Thank you for that - I will read through all these comments :)
|
This functionality has been released in v2.1.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
For issue: #516
This flag allows for terraform to interact with a list of currently existing groups within AAD for use in other environments (EG. Pagerduty).
It returns the same as other flags/inputs for the datasource and checks to ensure that a number greater than 0 are returned (otherwise an error is thrown in line with logic elsewhere)