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

.github: Fix error triggered by large comments #16360

Merged
merged 1 commit into from May 28, 2021

Conversation

pchaigno
Copy link
Member

@pchaigno pchaigno commented May 28, 2021

To ensure random pull request comments don't cancel ongoing workflows, commit c569ead (".github: Fix concurrency group for comment-triggered workflows") appended the comment message to the concurrency group name. Unfortunately, that results in an error when the comment is too large:

The maximum allowed memory size was exceeded while evaluating the following expression

This pull request fixes that error by simply appending trigger-phrase if the comment is one of the allowed trigger phrases and appending nothing otherwise. That way we avoid appending the whole comment to the concurrency group name, but still get a different concurrency group name for random comments and trigger comments.

This change was tested on a toy repository with both trigger phrases and a large comment.

Fixes: #16310.

To ensure random pull request comments don't cancel ongoing workflows,
commit c569ead (".github: Fix concurrency group for comment-triggered
workflows") appended the comment message to the concurrency group name.
Unfortunately, that results in an error when the comment is too large:

    The maximum allowed memory size was exceeded while evaluating the following expression

This commit fixes that error by simply appending 'trigger-phrase' if the
comment is one of the allowed trigger phrases and appending nothing
otherwise. That way we avoid appending the whole comment to the
concurrency group name, but still get a different concurrency group name
for random comments and trigger comments.

Fixes: c569ead (".github: Fix concurrency group for comment-triggered workflows")
Reported-by: Nicolas Busseneau <nicolas@isovalent.com>
Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno added kind/bug/CI This is a bug in the testing code. area/CI Continuous Integration testing issue or flake release-note/ci This PR makes changes to the CI. labels May 28, 2021
@pchaigno pchaigno requested review from a team as code owners May 28, 2021 17:06
@pchaigno pchaigno requested review from nbusseneau and aanm May 28, 2021 17:06
@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 28, 2021
@aanm aanm merged commit 6440f29 into cilium:master May 28, 2021
@pchaigno pchaigno deleted the fix-concurrency-group-error branch May 28, 2021 22:16
@pchaigno
Copy link
Member Author

pchaigno commented Jun 1, 2021

I had forgotten the backport label here. I'll make it in the next batch of backports, which I think is fine given it's not a critical error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake kind/bug/CI This is a bug in the testing code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants