-
Notifications
You must be signed in to change notification settings - Fork 101
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
Add no-code endpoint for fetching registry module variables. #979
Conversation
4a7bf25
to
966f2a1
Compare
// NOTE: At the time of authoring this comment, the API endpoint to fetch | ||
// registry module variables does not support pagination. This field is | ||
// included to satisfy jsonapi unmarshaler implementation here: | ||
// https://github.com/hashicorp/go-tfe/blob/3d29602707fa4b10469d1a02685644bd159d3ccc/tfe.go#L859 |
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 this could be a bug in the unmarshaler implementation. In the case of the change in this PR, Items
is a slice, and in the unmarshalResponse
function, it should be handled by jsonapi.UnmarshalManyPayload
. However, without the *Pagination
field of this struct present (which should not actually be here, because the backend service doesn't actually return a *Pagination
) in the response), the conditional here sends the request into the jsonapi
handler UnmarshalPayload
, which is documented to support "single request payloads only", leading an attempt to decode a slice to fail.
For now, just including the *Pagination
field here is a workaround. But if this issue is fixed, then it can be removed (or if pagination is implemented on the backend, it should be kept).
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.
This is a great callout, I'll create a ticket to look into this. 👍
9522682
to
a0e1538
Compare
a0e1538
to
3eae2c2
Compare
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.
One minor change to the changelog, a few extra comments and you're good to go :)
// NOTE: At the time of authoring this comment, the API endpoint to fetch | ||
// registry module variables does not support pagination. This field is | ||
// included to satisfy jsonapi unmarshaler implementation here: | ||
// https://github.com/hashicorp/go-tfe/blob/3d29602707fa4b10469d1a02685644bd159d3ccc/tfe.go#L859 |
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.
This is a great callout, I'll create a ticket to look into this. 👍
@@ -160,6 +202,14 @@ type RegistryNoCodeModuleReadOptions struct { | |||
Include []RegistryNoCodeModuleIncludeOpt `url:"include,omitempty"` | |||
} | |||
|
|||
type RegistryNoCodeModuleReadVariablesOptions struct { |
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 GODOC comment here as well
d77a247
to
e9a34c2
Compare
Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes. |
Description
This PR adds support to go-tfe for the HCP Terraform endpoint to retrieve a no-code module's variables.
Testing plan
nocode-abcdefgh12345678
NO_CODE_MODULE_ID
to be that ID.go test -run TestRegistryNoCodeModule -v ./...
, with the other necessary testing environment variables.Test Output