-
-
Notifications
You must be signed in to change notification settings - Fork 637
replace Imgur with GitHub Release Assets for screenshot hosting #3573
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
replace Imgur with GitHub Release Assets for screenshot hosting #3573
Conversation
Fixes AppImage#3548 ## Problem Imgur API returns "Too Many Requests" errors (code 1025), blocking successfully tested applications from being merged. ## Solution Replace Imgur with GitHub Release Assets: - Screenshots are uploaded to a draft release named 'ci-screenshots' - Inline images displayed in PR comments - Works for PRs from external contributors using workflow_run trigger
|
@probonopd |
- Add step to sanitize screenshot filenames before upload - Remove invalid characters (: * ? < > |) from app names - Fixes error with apps like 'fre:ac' containing colons - Update publish workflow to handle flat file structure
29420f9 to
36d2f39
Compare
| github.event.workflow_run.conclusion == 'success' && | ||
| github.event.workflow_run.event == 'pull_request' | ||
| github.event.workflow_run.event == 'pull_request' && | ||
| github.repository == 'AppImage/appimage.github.io' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This prevents workflow execution in forks - ensures releases and PR comments
are only created in the upstream repository.
| COUNT=0 | ||
| FALLBACK="run-${{ github.event.workflow_run.id }}" | ||
| for f in out/screenshots-upload/*.png; do | ||
| for f in out/*.png; do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
screenshots are now extracted directly to out/
(matching the actual artifact structure)
| - name: Ensure draft release exists | ||
| - name: Ensure release exists (published) | ||
| shell: bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
publish release (not draft) so that URLs render as inline images in PR comments, not just clickable links
|
Thank you very much @AshBuk. Let's try this out. It'd be even more elegant if we wouldn't need artifacts or releases for this, but I understand GitHub doesn't allow for posting comments in PRs with pictures via API directly yet. |
|
Hello @AshBuk could you please have a look as to why no screenshot has been posted to Looking at the Release, screenshots seem to have been taken. Thanks! |
|
Hi @probonopd ! check #3577 If permissions are enabled but the bot still doesn’t run in the upstream repo, I’ll dig into it tomorrow and will try provide a fix. |
Fixes #3548
Problem
Imgur API returns "Too Many Requests" errors (code 1025), blocking successfully tested applications from being merged.
Solution
Replace Imgur with GitHub Release Assets:
Workflow
Security
1. Separation of contexts:
2. Does not execute code from PR in privileged context:
Reference: Keeping your GitHub Actions and workflows secure: Preventing pwn requests