Skip to content

[terraform] Cache client-side timeouts when a remote host is unreachable#5407

Merged
Nishnha merged 4 commits intomainfrom
brrygrdn/cache-timeouts-for-terraform
Aug 17, 2022
Merged

[terraform] Cache client-side timeouts when a remote host is unreachable#5407
Nishnha merged 4 commits intomainfrom
brrygrdn/cache-timeouts-for-terraform

Conversation

@brrygrdn
Copy link
Copy Markdown
Contributor

@brrygrdn brrygrdn commented Jul 21, 2022

Follows up on #5142

This PR switches Terraform over to use the Dependabot::RegistryClient so it benefits from caching of unreachable hosts.

Since this gem implements Dependabot::Terraform::RegistryClient, I incorporated the caching client as a base class. I have one question about the existing implementation that might require some extra changes I walked that back as it just creates problems with the use of class variables and there's not a lot gained by inheritance.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I found it curious that the file parser doesn't already use Dependabot::Terraform::RegistryClient, but at a cursory read it seems like it is always going to be making unauthenticated calls so this is likely working as intended.

@jurre am I correct in thinking we intentionally do not use the terraform client here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's been a minute since I've worked on this, but I do think you're right yes

@brrygrdn brrygrdn force-pushed the brrygrdn/cache-timeouts-for-terraform branch from 6bc7d5b to 1583918 Compare July 21, 2022 14:57
@brrygrdn brrygrdn marked this pull request as ready for review July 26, 2022 11:41
@brrygrdn brrygrdn requested a review from a team as a code owner July 26, 2022 11:41
Copy link
Copy Markdown
Member

@Nishnha Nishnha left a comment

Choose a reason for hiding this comment

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

#get_proxied_source is being duplicated here since it was moved to the RegistryClient in a more recent PR

Copy link
Copy Markdown
Contributor

@landongrindheim landongrindheim left a comment

Choose a reason for hiding this comment

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

Thanks 🎉

@Nishnha Nishnha merged commit bd0d43e into main Aug 17, 2022
@Nishnha Nishnha deleted the brrygrdn/cache-timeouts-for-terraform branch August 17, 2022 21:33
@pavera pavera mentioned this pull request Aug 23, 2022
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.

5 participants