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

Caching for Terraform! #21221

Merged
merged 11 commits into from
Aug 4, 2024
Merged

Conversation

lilatomic
Copy link
Contributor

@lilatomic lilatomic commented Jul 28, 2024

This MR adds caching for Terraform! specifically the providers, which is the thing you need to download. It uses the provider cache.

Pulling in "hashicorp/azurerm" "hashicorp/azuread" (total 200M) goes from 22s to 2s (33s to 13s including pants startup).

@lilatomic lilatomic added category:new feature backend: Terraform Terraform backend-related issues labels Jul 28, 2024
@lilatomic lilatomic requested review from tdyas and alonsodomin July 28, 2024 01:46
@lilatomic lilatomic force-pushed the terraform/better-cache branch from 091b1a2 to c7f1b46 Compare July 28, 2024 01:58
@lilatomic lilatomic marked this pull request as ready for review July 28, 2024 01:58
src/python/pants/backend/terraform/dependencies_test.py Outdated Show resolved Hide resolved
src/python/pants/backend/terraform/tool.py Outdated Show resolved Hide resolved

path = []
user_path = env.get("PATH")
if user_path is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Edge case, but you probably don't want to append a blank PATH value either. Maybe just use if user_path: here then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point! We should centralise merging envvars at some point, I think there are a few subsystems that allow a passthrough of envvars in some way

Copy link
Contributor

@tdyas tdyas Aug 4, 2024

Choose a reason for hiding this comment

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

I added a common env vars helper in the adhoc / shell backends via the prepare_env_vars function.

Copy link
Contributor

Choose a reason for hiding this comment

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

Parts of that method might be useful elsewhere in the future.

@lilatomic lilatomic force-pushed the terraform/better-cache branch from 23644ce to 6a77c94 Compare August 4, 2024 03:30
@lilatomic lilatomic force-pushed the terraform/better-cache branch from 6a77c94 to 5ea8e70 Compare August 4, 2024 04:24
@lilatomic lilatomic merged commit 459d39b into pantsbuild:main Aug 4, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Terraform Terraform backend-related issues category:new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants