Skip to content
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

List Variable Sets associated with workspace #551

Closed
wants to merge 2 commits into from

Conversation

sebasslash
Copy link
Contributor

Description

These changes build off of #520, namely I'm making a slight modification on which interface this method belongs to.

In the original PR, the method to list variable sets for a workspace was VariableSets.ListForWorkspaces() and I've changed it to: Workspaces.ListVariableSets(). Given we typically have children relationships exposed as part of the parent interface, i.e Workspaces.ListSomeChildren(), I felt this API was more natural in describing the one-to-many relationship.

It is worth noting that this endpoint is documented under our Variable Set API, however the path is /workspaces/:workspace_id/varsets so I felt like this is ultimately part of the Workspaces API.

I've also added the applyVariableSetToWorkspace() test helper, which was missing in the original PR.

Besides this, I've kept the original implementation from @tstapler (Great work if you see this!)

Testing plan

go test -v ./... -run TestWorkspacesListVariableSets -tags=integration

External links

@sebasslash sebasslash requested a review from a team as a code owner October 17, 2022 17:04
@sebasslash sebasslash force-pushed the sebasslash/list-workspace-variable-sets branch from 7ee018c to d178306 Compare October 17, 2022 17:09
Copy link
Collaborator

@brandonc brandonc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my preference is to attach this to the VariableSets interface. Here is my rationale: Nearly everything belongs to a workspace, but the non-core interface we offer in go-tfe seems to scoped to workspace settings like tags, ssh keys, and remote state consumers.

Other things associated to workspaces (config versions, state versions, notifications, variables, run tasks) don't extend the workspace interface to add support.

@sebasslash
Copy link
Contributor Author

sebasslash commented Oct 17, 2022

That's a fair point. I'll close this PR and rebase on the original.

@sebasslash sebasslash closed this Oct 17, 2022
@annawinkler annawinkler deleted the sebasslash/list-workspace-variable-sets branch February 2, 2023 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants