-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Ensure terraform_version is downloaded when using non built-in step. Fixes #675 #722
Conversation
Codecov Report
@@ Coverage Diff @@
## master #722 +/- ##
==========================================
- Coverage 72.45% 72.37% -0.08%
==========================================
Files 61 61
Lines 4679 4695 +16
==========================================
+ Hits 3390 3398 +8
- Misses 1039 1044 +5
- Partials 250 253 +3
Continue to review full report at Codecov.
|
5829859
to
6190d38
Compare
Thanks @maximede, can you remove the binary that was committed here? We should add that to our .gitignore, simple easy to accidentally commit that. |
Good catch, sorry I missed the file. |
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.
Thanks again, can you squash to a single commit before I merge?
c.versionsLock.Lock() | ||
_, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir) | ||
c.versionsLock.Unlock() | ||
if err != nil { |
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.
can just return err
here
|
||
err := r.TerraformExecutor.EnsureVersion(ctx.Log, tfVersion) | ||
if err != nil { | ||
err = fmt.Errorf("%s: Downloading terraform Version %s", err, tfVersion.String()) |
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.
Use errors.Wrapf
, use all lowercase:
err = errors.Wrapf(err, "downloading terraform version %s", tfVersion.String()
err := r.TerraformExecutor.EnsureVersion(ctx.Log, tfVersion) | ||
if err != nil { | ||
err = fmt.Errorf("%s: Downloading terraform Version %s", err, tfVersion.String()) | ||
ctx.Log.Debug("error: %s", err) |
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.
I see that this is a Debug
log below as well but I think this makes more sense as an Err
log
No description provided.