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.

Sign up for GitHub

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

Jump to bottom

fix: avoid generating many github comments on plan, apply #4586

Closed
wants to merge 4 commits into from
Closed

fix: avoid generating many github comments on plan, apply #4586

wants to merge 4 commits into from

Conversation

benoittoulme
Copy link
Contributor

@benoittoulme benoittoulme commented May 21, 2024 •
edited
Loading

what

  • Only print the end of a terraform plan or apply if the rendered text will create more than 500000 characters (8 github comments).
  • Otherwise, print only the end of the output (to show the summary of the plan, for example) and invite the user to look at the atlantis job page.

why

  • Many github comments are hard to read, cause slow UI rendering, and may make github throttle atlantis depending on how it is configured.

tests

  • I have tested my changes by deploying on our atlantis server, and we do have a few deployments that are very, very chatty.

references

@benoittoulme
only print the end of a terraform plan or apply if the rendered text …
3b0e98b 
…will create more than 500000 characters (8 github comments)
@benoittoulme benoittoulme requested review from a team as code owners May 21, 2024 21:43
@benoittoulme benoittoulme requested review from jamengual, lukemassa and nitrocode and removed request for a team May 21, 2024 21:43
@benoittoulme benoittoulme changed the title avoid generating many github comments avoid generating many github comments on plan, apply May 21, 2024
@chenrui333 chenrui333 changed the title avoid generating many github comments on plan, apply fix: avoid generating many github comments on plan, apply May 21, 2024
@chenrui333
Copy link
Member

@benoittoulme Thanks for the PR. Can you add some test for such change?

@benoittoulme benoittoulme mentioned this pull request May 22, 2024
Merged
@jamengual jamengual added the waiting-on-response Waiting for a response from the user label May 24, 2024
chenrui333
chenrui333 approved these changes Jun 3, 2024
View reviewed changes
Copy link
Member

@chenrui333 chenrui333 left a comment

Choose a reason for hiding this comment

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

lgtm

@benoittoulme
Copy link
Contributor Author

Sorry for the delay, I am trying to spend some time to work on this issue this week.
I found this other PR (#3905) and have proposed to help implementation with the other PR instead of mine, as I think the proposed solution is better. see my comment #3905 (comment)

@chenrui333
Copy link
Member

@chenrui333 chenrui333 marked this pull request as draft June 4, 2024 14:53
@chenrui333
Copy link
Member

move this to the draft mode

@benoittoulme
Copy link
Contributor Author

With #3905 merged, we do not need this pr anymore. Closing.

@benoittoulme benoittoulme deleted the avoid-many-comments branch June 27, 2024 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chenrui333 chenrui333 chenrui333 approved these changes

@jamengual jamengual Awaiting requested review from jamengual jamengual is a code owner automatically assigned from runatlantis/maintainers

@lukemassa lukemassa Awaiting requested review from lukemassa lukemassa is a code owner automatically assigned from runatlantis/core-contributors

@nitrocode nitrocode Awaiting requested review from nitrocode nitrocode is a code owner automatically assigned from runatlantis/core-contributors

Assignees
No one assigned
Labels
waiting-on-response Waiting for a response from the user
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benoittoulme @chenrui333 @jamengual