Skip to content

Handle long diffs gracefully in compare-ds github action#8041

Merged
matejak merged 1 commit intoComplianceAsCode:masterfrom
ggbecker:limit-compare-ds-diff-size
Jan 10, 2022
Merged

Handle long diffs gracefully in compare-ds github action#8041
matejak merged 1 commit intoComplianceAsCode:masterfrom
ggbecker:limit-compare-ds-diff-size

Conversation

@ggbecker
Copy link
Copy Markdown
Member

Description:

  • Handle long diffs gracefully in compare-ds github action

Rationale:

  • Apparently it's not possible to place comments larger than 65k characters so we have to limit them.

Tested in ggbecker#26 (comment)
https://github.com/ggbecker/content/runs/4762507114?check_suite_focus=true

Co-authored with @matejak

@ggbecker ggbecker added this to the 0.1.60 milestone Jan 10, 2022
@ggbecker ggbecker requested a review from matejak January 10, 2022 14:38
Copy link
Copy Markdown
Member

@matejak matejak left a comment

Choose a reason for hiding this comment

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

The comments needs a typo fixed, otherwise LGTM!

Comment thread .github/workflows/compare-ds.yaml Outdated
issue-number: ${{ github.event.pull_request.number }}
body: |
This datastream diff is auto generated by the check `Compare DS/Generate Diff`.
Due to the excessive size of the diff, it has been trimmed to fit the 4096-character limit.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The limit is no longer 4096 characters.
It is probably based on 16-bit integer (0 to 65535), so setting the limit body to 65000 is sufficient - the surrounding text can't get longer that easily.
Initially I thought that 8000+ characters is too much, but I don't think so any more, the diff in the example PR is perfectly usable.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed by amending the commit. The limit is somehow connected to: actions/runner-images#3257 but I haven't really spend too much time investigating it. I believe the 65k chars is enough for our purposes.

@ggbecker ggbecker force-pushed the limit-compare-ds-diff-size branch from c469fc6 to 5177f7b Compare January 10, 2022 15:39
@matejak matejak merged commit afc3a60 into ComplianceAsCode:master Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants