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

Enable alternative reading of a registry module by its ID #988

Merged
merged 6 commits into from
Oct 22, 2024

Conversation

dsa0x
Copy link
Contributor

@dsa0x dsa0x commented Oct 17, 2024

Description

This enables an alternative way to read a registry module by its ID instead of its address (namespace/name/system). When the external ID is given, all other attributes are ignored.

Testing plan

  • Create a token in your HCP TF instance
  • Add the TFE_ADDRESS and TFE_TOKEN variables to your environment
  • Running go test -v -run ^TestRegistryModulesRead$ github.com/hashicorp/go-tfe should succeed.

External links

Output from tests

Including output from tests may require access to a TFE instance. Ignore this section if you have no environment to test against.

$ TFE_ADDRESS="https://example" TFE_TOKEN="example" go test -v -run ^TestRegistryModulesRead$ github.com/hashicorp/go-tfe
=== RUN   TestRegistryModulesRead/with_a_external_ID_field_for_private_module
=== RUN   TestRegistryModulesRead/with_a_external_ID_field_for_private_module/permissions_are_properly_decoded
=== RUN   TestRegistryModulesRead/with_a_external_ID_field_for_private_module/timestamps_are_properly_decoded
...
    --- PASS: TestRegistryModulesRead/with_a_external_ID_field_for_private_module (0.21s)
        --- PASS: TestRegistryModulesRead/with_a_external_ID_field_for_private_module/permissions_are_properly_decoded (0.00s)
        --- PASS: TestRegistryModulesRead/with_a_external_ID_field_for_private_module/timestamps_are_properly_decoded (0.00s)
...
PASS
ok      github.com/hashicorp/go-tfe     3.926s

...

@dsa0x dsa0x force-pushed the sams/TFECO-6399-read-by-external-id branch from 1fe6259 to ed36a7a Compare October 17, 2024 06:48
@dsa0x dsa0x requested review from a team October 17, 2024 07:12
@dsa0x dsa0x changed the title Enable alternative registry module reading by its external ID Enable alternative reading of a registry module by its external ID Oct 17, 2024
tfe.go Outdated Show resolved Hide resolved
registry_module.go Outdated Show resolved Hide resolved
tfe.go Outdated Show resolved Hide resolved
@dsa0x dsa0x changed the title Enable alternative reading of a registry module by its external ID Enable alternative reading of a registry module by its ID Oct 17, 2024
@dsa0x dsa0x force-pushed the sams/TFECO-6399-read-by-external-id branch from 55cb689 to 414e429 Compare October 17, 2024 20:44
@dsa0x dsa0x requested a review from brandonc October 17, 2024 21:18
var u string
if moduleID.ID == "" {
if moduleID.RegistryName == "" {
log.Println("[WARN] Support for using the RegistryModuleID without RegistryName is deprecated as of release 1.5.0 and may be removed in a future version. The preferred method is to include the RegistryName in RegistryModuleID.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know you didn't add this, but registry_module.go seems to be the only file in go-tfe which is using the log package, and, as a library, probably shouldn't be logging to stderr. It would be nice if you could go ahead and remove all these log.Print calls from this file, but I'll take care of it if you don't feel like waiting for another review!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. I will go ahead and merge this. Is there a convention employed in go-tfe for including deprecated messages?. Perhaps we could just remove them entirely, and have the comments indicate the deprecated attributes.

@dsa0x dsa0x merged commit fc80091 into main Oct 22, 2024
7 checks passed
@dsa0x dsa0x deleted the sams/TFECO-6399-read-by-external-id branch October 22, 2024 06:38
Copy link

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.

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.

2 participants