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

Optionally Limit GitHub Comment Creation #3322

Open
1 task done
schallert opened this issue Apr 12, 2023 · 1 comment
Open
1 task done

Optionally Limit GitHub Comment Creation #3322

schallert opened this issue Apr 12, 2023 · 1 comment
Labels
feature New functionality/enhancement

Comments

@schallert
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.

Describe the user story

As a developer, I would love to always write bug-free Terraform modules, but sometimes make mistakes in them. Depending on the type of error and the size of the environment(s) the module is used in, when using Atlantis, it may result in many GitHub comments being added to a PR. The volume of comments may clutter the PR, result in many notifications, or even breach GitHub rate limits (if not using the GitHub app approach).

Describe the solution you'd like

Often, only the later lines in the plan are relevant to the error. Most of the output is often refreshing state for resources in the environment.

Ideally, Atlantis would figure out the relevant part of the the plan output or only show the unique errors. However, I can see how that would be difficult, and I can imagine something much easier: if a plan output will be more X comments, I would like a configuration option (e.g. --max-comments-per-plan=N) that would limit Atlantis to only the last N comments of a plan output. This would put the user in control of the cap on the number of comments for a plan execution across multiple environments (if the flag is N and the user has M environments, the number of comments would be no more than N * M).

From my experience, I think in the overwhelming majority of cases where a user enabled this flag, they would still see relevant errors even if some comments were cut off.

Also, while I've used GitHub as an example here, this approach would be generic across all VCS providers.

If the general behavior described above is appealing, once we agree on the semantics I would be happy to implement this feature.

Describe the drawbacks of your solution

The main drawback I can see is cases where some unique errors are hidden by this behavior. For example:

  • If you have a plan with 50 instances of error A, and 1 instance of error B.
  • The plan failure would result in 5 GitHub comments, with error A and B in the first comment, and only error B in the following 4 comments.
  • The max comments cap is set to 4.

In such a scenario, you would not be able to address every error in the PR based on the comments alone. However, the fact that the user could still see the full logs in the Atlantis UI helps mitigate the issue. Finally, I believe that since this flag would be opt-in vs. on by default, the risk of such edge cases is limited to the broader community.

Describe alternatives you've considered

As mentioned above, it would be great if the number of max comments could be based on unique errors, or some other structured interpretation of the plan output. However, I believe this would be more difficult to implement correctly while giving the user confidence all errors were captured. It's also possible this could miss cases of comment clutter / GitHub rate limits. For example: if the plan contained thousands of unique errors per-plan across many environments, the issue would still pop up.

@schallert schallert added the feature New functionality/enhancement label Apr 12, 2023
@DanielCastronovo
Copy link

It could be interesting to remove the rate limit from Github

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality/enhancement
Projects
None yet
Development

No branches or pull requests

2 participants