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

Add ability to skip cloning if no Terraform is modified #973

Closed
wants to merge 3 commits into from

Conversation

cket
Copy link
Contributor

@cket cket commented Apr 3, 2020

Related to #967. Adds skip-clone-no-tf flag to control this behavior. I'm not sure if this will exactly satisfy the requirements in the ticket since it does not inspect atlantis.yaml but should be a step in the right direction

A couple benefits to this addition:

  • not specific to any VCS platform
  • improved performance for non-TF PRs - would help Atlantis scale with larger amounts of non-TF PRs
  • prevents certain Atlantis errors on non-TF PRs by avoiding the need for the lock in those situations (workspace is currently locked by another command that is running for this pull request)

Let me know if I'm missing anything or if this is helpful

@codecov
Copy link

codecov bot commented Apr 3, 2020

Codecov Report

Merging #973 into master will increase coverage by 0.04%.
The diff coverage is 84.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #973      +/-   ##
==========================================
+ Coverage   72.12%   72.16%   +0.04%     
==========================================
  Files          65       65              
  Lines        5316     5325       +9     
==========================================
+ Hits         3834     3843       +9     
  Misses       1186     1186              
  Partials      296      296
Impacted Files Coverage Δ
server/user_config.go 100% <ø> (ø) ⬆️
cmd/server.go 79.26% <ø> (ø) ⬆️
server/server.go 63.66% <100%> (+0.1%) ⬆️
server/events/project_finder.go 94.56% <100%> (+0.18%) ⬆️
server/events/project_command_builder.go 82.5% <77.77%> (+0.37%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 548fe5f...889e60e. Read the comment docs.

@lkysow
Copy link
Member

lkysow commented Apr 16, 2020

Hey Christopher, thanks for the PR but I'm not sure this is the best approach that will solve the whole problem for all atlantis users.

Many Atlantis users use the atlantis.yaml file to determine which file patterns will trigger a plan. By only looking at .tf files these users aren't supported.

I looked into doing a clone into a temp directory and then performing the detection there before moving into the working directory but we can't simply move the cloned directory and clobber the existing working directory because we actually expect it to persist through atlantis commands run on the same pull request so that the .tfplan files aren't deleted.

Thus we'd need to clone it first into the temp dir, run our checks and then re-clone it into the proper working dir. This seems like a smell that this also isn't the right approach.

I think we either need to support queuing or go the route that @timoguin talked about in the ticket where we make an api call to get the contents of atlantis.yaml.

@lkysow lkysow added the waiting-on-response Waiting for a response from the user label Apr 16, 2020
@cket
Copy link
Contributor Author

cket commented Apr 21, 2020

Got it. I think a call to view atlantis.yaml would be preferable so we don't clone if we don't have to. It sounds like that is possible for github at least but I'm not sure if other vcs have that capability

@lkysow
Copy link
Member

lkysow commented Apr 21, 2020

It sounds like that is possible for github at least but I'm not sure if other vcs have that capability

For those VCS's we can just keep the existing logic and always clone.

@lkysow
Copy link
Member

lkysow commented May 25, 2020

Closing in favour of #1040 which supports downloading the atlantis.yaml file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-response Waiting for a response from the user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants