-
Notifications
You must be signed in to change notification settings - Fork 69
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
Support 'offline mode': do not download terraform binary #124
Conversation
…from PATH Signed-off-by: Gennady Lipenkov <xgen@yandex-team.ru>
@bflad Please check this PR) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @GennadySpb 👋 Thank you for submitting this. As discussed in #77 (which I think we should give @syndicut an opportunity to adjust/submit first), I think an ideal implementation here would be to unilaterally check for a Terraform binary first, then fall back to downloading the latest version. If folks desire a way to use an exact binary, we can provide a separate flag or environment variable.
internal/provider/generate.go
Outdated
src = append(src, &releases.ExactVersion{ | ||
Product: product.Terraform, | ||
Version: version.Must(version.NewVersion(defaultTerraformVersion)), | ||
InstallDir: tmpDir, | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommend switching this to checkpoint.LatestVersion
, instead of keeping the hardcoded Terraform version.
…wnloading the latest version; also support set exact version to download. Signed-off-by: Gennady Lipenkov <xgen@yandex-team.ru>
So, make refactor to implement described behavior. Also support set exact Terraform version by arg |
Thanks for the update, @GennadySpb 👍 We'll see if @syndicut responds in the next week or so and if not, we can move forward with something like this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor things, otherwise looking good to me.
I don't mind merging @GennadySpb's PR instead of mine |
Co-authored-by: Brian Flad <bflad417@gmail.com>
Co-authored-by: Brian Flad <bflad417@gmail.com>
Co-authored-by: Brian Flad <bflad417@gmail.com>
@bflad accept all your changes ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thank you @GennadySpb 🚀 Verified using terraform-provider-scaffolding
:
$ go mod edit -replace=github.com/hashicorp/terraform-plugin-docs=/Users/bflad/src/github.com/hashicorp/terraform-plugin-docs
$ go mod tidy
$ go generate ./...
rendering website for provider "terraform-provider-scaffolding"
exporting schema from Terraform
compiling provider "scaffolding"
using Terraform CLI binary from PATH if available, otherwise downloading latest Terraform CLI binary
running terraform init
getting provider schema
rendering missing docs
generating missing resource content
generating template for "scaffolding_resource"
generating missing data source content
generating template for "scaffolding_data_source"
generating missing provider content
generating template for "terraform-provider-scaffolding"
rendering static website
cleaning rendered website dir
rendering templated website to static markdown
rendering "data-sources/data_source.md.tmpl"
rendering "index.md.tmpl"
rendering "resources/resource.md.tmpl"
So nice saving the download time each execution. 😄
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Signed-off-by: Gennady Lipenkov xgen@yandex-team.ru