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

feat(terraform-provider): Add secondary release repo #6513

Merged
merged 7 commits into from Jun 22, 2020

Conversation

secustor
Copy link
Collaborator

This PR adds releases.hashicorp.com as secondary repo for builds.

The primary source is still registry.terraform.io as it has more meta information e.g. source code repositories, changelogs, ...

Closes #5065

@rarkins
Copy link
Collaborator

rarkins commented Jun 16, 2020

Hi @secustor, can you refactor this a little before we focus on tests?

  1. Move the existing registry logic out of getReleases() and into its own function, e.g. queryRegistry()
  2. Use a single cache entry that covers both

e.g. the logic becomes

  • if cached entry exists then return it
  • look up registry
  • if registry fails, look up releases
  • cache result and return it

@secustor secustor force-pushed the add-secondary-terraform-source branch 3 times, most recently from 6e580c9 to 961fdaf Compare June 18, 2020 20:27
@secustor secustor force-pushed the add-secondary-terraform-source branch from 961fdaf to c4582ff Compare June 18, 2020 20:32
@secustor secustor requested a review from rarkins June 18, 2020 20:41
@secustor
Copy link
Collaborator Author

secustor commented Jun 18, 2020

Rebased and updated to the new mocking setup.

Seems like the second nock scope is breaking the other unit tests.
Looks like a edge case which is not covered in the overloading.

@JamieMagee
Copy link
Contributor

@secustor I fixed a couple of mocks you were missing. With the fallback logic you implemented, we need to mock both possible requests.

@secustor secustor marked this pull request as ready for review June 19, 2020 07:29
@rarkins
Copy link
Collaborator

rarkins commented Jun 20, 2020

@secustor are there any cases where results could be found in both registries and we'd prefer to combine results?

@secustor
Copy link
Collaborator Author

@JamieMagee Thx for your support.

@rarkins Not really the secondary repo contains only the available version and cpu architectures for the providers.

@rarkins
Copy link
Collaborator

rarkins commented Jun 20, 2020

Can you check out the recent commit I made which adds the concept registryStrategy = ‘hunt’? It simplifies the datasource implementations by pushing some logic into the index dispatcher.

Let me know if you’re interested to refactor as part of this PR or you don’t mind me refactoring it after.

@lgtm-com
Copy link

lgtm-com bot commented Jun 20, 2020

This pull request introduces 2 alerts when merging d8dfea2 into 7e51c90 - view on LGTM.com

new alerts:

  • 2 for Incomplete URL substring sanitization

@lgtm-com
Copy link

lgtm-com bot commented Jun 20, 2020

This pull request introduces 2 alerts when merging cdf1516 into 7e51c90 - view on LGTM.com

new alerts:

  • 2 for Incomplete URL substring sanitization

@secustor
Copy link
Collaborator Author

@rarkins I have refactored the changes to this new strategy system.

@rarkins rarkins merged commit 1a066c6 into renovatebot:master Jun 22, 2020
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 21.14.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@secustor secustor deleted the add-secondary-terraform-source branch June 22, 2020 10:04
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

google-beta terraform provider doesn't get upgraded
4 participants