Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Manage Variable Sets #452
Manage Variable Sets #452
Changes from 24 commits
8e4fd3e
065b3b3
e69468b
925bdd6
376ab18
6c06a70
5b99284
06f5e5d
2b22db2
f1da8bc
730bce2
bfd3f61
917a235
59818cc
02bd402
379451a
690c2cf
100c706
5d2f599
362e6bb
b5ee652
e5b056b
3b32606
b67577b
8533d09
b5d4a74
5b900b8
4e3bd4d
4c76a56
1064a22
873a7ac
9e0aa84
566ebe5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 might be wrong on this, but from what I've read in the API docs workspace and variable IDs are included in the response body for the list endpoint? So you can simply loop through the
Workspaces
field without the need to make an extra API call: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.
My thinking here was to not put the extra load of looking up and serializing all the vars and workspaces until we know which one we care about. There are realistic scenarios where I believe the List call could become quite slow if asked to collect all that extra data. Happy to chat more about it if that seems unfounded.
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.
Let's add a test check for the
variable_ids
attribute as well. However, we'd typically structure tests as follows:TestAccTFEVariableSetsDataSource_basic
: Test the required attributes onlyTestAccTFEVariableSetsDataSource_full
: Test the required and optional attributes. Each attribute has its own test case. For attributes that are aggregate types we test for two things: the length/number-of-keys, and that each element matches the desired IDThere 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.
Since the ids are determined at run time how can I reference them from inside the test func declaration? Looking for prior art and coming up short.
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.
Let's add a
ValidateFunc
forworkspace_id
andvariable_set_id
(We don't do this consistently throughout the provider and it would be a good convention to establish):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.
Do you recommend this for the
resource*
definitions as well?