-
Notifications
You must be signed in to change notification settings - Fork 458
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
fix(cli): dont upload .terraform folders #758
Conversation
8a9fa9e
to
4c3b696
Compare
4c3b696
to
255b3dd
Compare
255b3dd
to
cb76c38
Compare
ddd92c7
to
34bdd73
Compare
f27e2c8
to
d3f3989
Compare
d3f3989
to
7d0e7b5
Compare
@@ -127,6 +130,8 @@ export class TerraformCloud implements Terraform { | |||
|
|||
this.configurationVersionId = version.id | |||
|
|||
this.removeLocalTerraformDirectory() |
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.
There are two modes for Terraform Cloud: local execution and remote execution. Could this possibly break local execution or be unwanted (as it destroys the local cache containing already initialized Terraform providers)? 🤔
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 would guess no as this is the terraform cloud implementation of this function. The Cli implementation would need to be able to deal with a blank state so it should work IMHO
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.
Yeah, I think this should be ok - since that's scoped to the stack anyway. Even if you had multiple stacks targeting local and cloud, it should still be ok.
@@ -127,6 +130,8 @@ export class TerraformCloud implements Terraform { | |||
|
|||
this.configurationVersionId = version.id | |||
|
|||
this.removeLocalTerraformDirectory() |
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.
Yeah, I think this should be ok - since that's scoped to the stack anyway. Even if you had multiple stacks targeting local and cloud, it should still be ok.
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 issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Closes #711