-
Notifications
You must be signed in to change notification settings - Fork 0
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
Support for Scopes #53
Conversation
scope_type = "fixed" | ||
display_name = "Fixed Scope" | ||
workspace_id = squaredup_workspace.application_workspace.id | ||
node_ids = [data.squaredup_nodes.acommon_node.node_properties[0].id, data.squaredup_nodes.api_node.node_properties[0].id] |
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.
What's with the node_properties[0]
- is this a Terraform thing? How many sets of properties will a node have? If always just the one perhaps we can hide this if not a Terraform "thing"
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 think node_properties
can be an array of multiple nodes if lookup on like 44-48 above returns more than one object.
A previous PR went in to allow lookup by SourceId to help prevent this, but it can occur with name lookups.
Is that correct @shaswot77?
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.
Thanks @P2P-Nathan. That is correct.
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 question was more about the choosing of the slightly confusing node_properties
variable name. Did we choose this or is it a Terraform thing? Where is it defined / chosen?
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.
No this was chosen. The reason I had used node_properties
is because the api returns these data in properties
object. There is also another properties
within the properties
object therefore I went for a slightly different name
|
||
type Scope struct { | ||
Name string `json:"name"` | ||
Version int `json:"version"` |
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.
What's this version, I don't see it referenced anywhere else?
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.
Version is used on the POST
body and without it the UI wasn't rendering as expected
scope_type = "fixed" | ||
display_name = "Fixed Scope" | ||
workspace_id = squaredup_workspace.application_workspace.id | ||
node_ids = [data.squaredup_nodes.acommon_node.node_properties[0].id, data.squaredup_nodes.api_node.node_properties[0].id] |
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 think node_properties
can be an array of multiple nodes if lookup on like 44-48 above returns more than one object.
A previous PR went in to allow lookup by SourceId to help prevent this, but it can occur with name lookups.
Is that correct @shaswot77?
Description
New resource to allow creating
advanced(Not supported in the UI)
,dynamic
andFixed
scopeChecklist:
bug-fix
,feature
, orenhancement