-
Notifications
You must be signed in to change notification settings - Fork 694
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
[BUG]: data.github_organization.my_org.users[*].id
is unexpectedly a Node ID
#2207
Comments
Hello! I was on the fence to open this as a bug vs a feature request. Everything is of course working, but the experience is inconsistent. Do not intend any offence by labelling it as a bug. 5 months back I attempted some refactoring of our terraform to remove what looked like unnecessary Yesterday I was revisiting some parts of that project and attempted the same refactoring, having forgotten about the limitation. Ended up with pretty well the same diff, and then had a good chuckle afterward. If From the resources/data sources we've used, I'm not much of a gopher, nor spent any time developing terraform providers, but I did take a stab at a proposal in #2206. Test weren't passing for me even on Hope this issue and proposal makes some sense - happy to discuss. |
Hello! First of all, no offense taken; there's a whole lot wrong with the provider, and I'm always happy to talk about ways to make it better, especially if you're willing to open a PR to make it better, that's my favorite thing. The ID thing I agree is annoying...my personal preference would be to expect ID to be the GitHub or database ID, and the GraphQL or Node ID to be The real issue that holds up a concentrated effort to change this is that breaking changes in Terraform are a pain; Hashicorp recommends we don't do more than one a year to minimize strain on consumers and we just recently did v6. I'm 👍 on the deprecation idea and your proposal seems reasonable, though realistically it'll be close to a year before that deprecation will be able to be carried out.
This isn't you; this is a symptom of the provider having bad test issues. On the main branch I see the following output for the relevant tests:
and the test is failing due to the following error: All of this is to say: I'm receptive to your ideas and I think you're on the right track. If you want to push them further, I'd be happy to review. Adding additional integration test cases for your new fields (or dare I say, fixing the currently broken "happy path" test) would be much appreciated. |
👍 Thanks for taking a look. I'll review the existing tests and see if they can't be improved before moving my existing PR out of draft state. |
Expected Behavior
I would expect the
id
field of theuser
objects provided ondata_source_github_organization
to be the integer id of the user, similar todata_source_github_user
: https://registry.terraform.io/providers/integrations/github/latest/docs/data-sources/user#idActual Behavior
The
id
field of theuser
object is the GraphQL Node Id, which is generally referred to asnode_id
, such as ondata_source_github_user
: https://registry.terraform.io/providers/integrations/github/latest/docs/data-sources/user#node_id and ondata_source_github_organization
itself: https://registry.terraform.io/providers/integrations/github/latest/docs/data-sources/organization#node_idTerraform Version
Terraform v1.7.5 on linux_amd64 + provider registry.terraform.io/integrations/github v6.2.0
Affected Resource(s)
Terraform Configuration Files
No response
Steps to Reproduce
No response
Debug Output
No response
Panic Output
No response
Code of Conduct
The text was updated successfully, but these errors were encountered: