Uh oh!
There was an error while loading. Please reload this page.
ADFA-3269 Share nightly release build with private Telegram channel - #1073
Conversation
…telegram channel; remove obsolete Greengeeks ssh setup
📝 WalkthroughRelease Notes - ADFA-3269Key Changes
Technical Details
Risks & Best Practices Notes
✓ Security Improvement: Removing SSH-based uploads in favor of Cloudflare R2 API authentication is a security best practice—eliminates SSH key management burden and reduces attack surface. WalkthroughThe release workflow replaces SSH-based artifact uploads with Cloudflare R2 uploads via a new Python script, removes SSH key setup logic, and adds Telegram notifications after the upload completes. The workflow maintains existing post-upload messaging while adding new cloud storage and messaging integrations. Changes
Sequence DiagramsequenceDiagram
participant GA as GitHub Actions
participant Script as cloudflare-r2-upload.py
participant R2 as Cloudflare R2
participant Telegram as Telegram API
GA->>GA: Build APK
GA->>Script: Execute with file path & variant
Script->>Script: Validate environment variables
Script->>Script: Initialize boto3 S3 client
Script->>R2: Upload APK with progress tracking
R2-->>Script: Upload complete
Script->>GA: Print completion status
GA->>Telegram: Send notification with APK link
Telegram-->>GA: Message delivered
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/release.yml (1)
588-594: Consider storingCLOUDFLARE_KEY_IDas a secret rather than a variable.While the Key ID is less sensitive than the Secret Access Key, it's still part of the credential pair. Storing it as a
secretinstead of avarwould provide additional protection (masked in logs, not visible in repository settings to users with lower permissions).env: CLOUDFLARE_ACCOUNT_ID: ${{ vars.CLOUDFLARE_ACCOUNT_ID }} - CLOUDFLARE_KEY_ID: ${{ vars.CLOUDFLARE_KEY_ID }}+ CLOUDFLARE_KEY_ID: ${{ secrets.CLOUDFLARE_KEY_ID }} CLOUDFLARE_SECRET_ACCESS_KEY: ${{ secrets.CLOUDFLARE_SECRET_ACCESS_KEY }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 588 - 594, The workflow step "Upload APK to Cloudflare R2" exposes CLOUDFLARE_KEY_ID as a repo variable; change the env binding to use a secret (secrets.CLOUDFLARE_KEY_ID) instead of vars.CLOUDFLARE_KEY_ID and add that Key ID to the repository secrets, leaving CLOUDFLARE_SECRET_ACCESS_KEY as secrets.CLOUDFLARE_SECRET_ACCESS_KEY; ensure the step still passes steps.find_apk.outputs.APK_PATH and matrix.variant into scripts/cloudflare-r2-upload.py unchanged.scripts/cloudflare-r2-upload.py (1)
73-79: Consider adding error handling around the upload operation.The
s3.upload_file()call can raise various boto3/botocore exceptions (e.g.,ClientError,NoCredentialsError, network errors). Currently, failures will produce raw tracebacks. Catching and logging a cleaner error message would improve debuggability in CI logs.Proposed enhancement
+from botocore.exceptions import ClientError+ print(f"Uploading {file_name} ({file_size / (1024*1024):.1f} MB) to R2...", flush=True) -s3.upload_file(file_path, BUCKET_NAME, file_name, **upload_kwargs)-print("Upload complete.", flush=True)+try:+ s3.upload_file(file_path, BUCKET_NAME, file_name, **upload_kwargs)+ print("Upload complete.", flush=True)+except ClientError as e:+ print(f"ERROR: Upload failed: {e}", file=sys.stderr)+ sys.exit(1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/cloudflare-r2-upload.py` around lines 73 - 79, Wrap the s3.upload_file call in a try/except block to catch boto3/botocore exceptions (e.g., botocore.exceptions.ClientError, NoCredentialsError, EndpointConnectionError, and a generic Exception fallback); reference the existing upload_kwargs, progress_callback, s3.upload_file, BUCKET_NAME, file_path and file_name to locate the call, log/print a clear error message including the exception details, and exit with a non-zero status (or re-raise) so CI sees the failure instead of a raw traceback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 711-713: The Telegram POST currently silences failures (curl -s)
so the workflow continues on HTTP errors; update the step that calls curl (the
command using TELEGRAM_TOKEN, TELEGRAM_EARLY_ACCESS_CHAT_ID, and MESSAGE) to
detect and surface non-2xx responses by either using curl's --fail/-f and -S
flags or capturing the HTTP status and response body and then failing the job
when the status is not 200/201; ensure errors are logged (include response body
or status) and the step exits non-zero so the workflow does not continue
silently on rate limits, invalid tokens, or network issues.
In `@scripts/cloudflare-r2-upload.py`:
- Around line 41-51: Check that the provided file_path exists before calling
os.path.getsize to avoid FileNotFoundError; specifically, after assigning
file_path from sys.argv and before file_size = os.path.getsize(file_path), add a
guard using os.path.exists(file_path) (or os.path.isfile) and if it does not
exist print a clear error to stderr mentioning the missing file_path and exit
with non-zero status so the script fails cleanly; keep the subsequent logic that
computes base_name, name_root, ext, and file_name unchanged.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 588-594: The workflow step "Upload APK to Cloudflare R2" exposes
CLOUDFLARE_KEY_ID as a repo variable; change the env binding to use a secret
(secrets.CLOUDFLARE_KEY_ID) instead of vars.CLOUDFLARE_KEY_ID and add that Key
ID to the repository secrets, leaving CLOUDFLARE_SECRET_ACCESS_KEY as
secrets.CLOUDFLARE_SECRET_ACCESS_KEY; ensure the step still passes
steps.find_apk.outputs.APK_PATH and matrix.variant into
scripts/cloudflare-r2-upload.py unchanged.
In `@scripts/cloudflare-r2-upload.py`:
- Around line 73-79: Wrap the s3.upload_file call in a try/except block to catch
boto3/botocore exceptions (e.g., botocore.exceptions.ClientError,
NoCredentialsError, EndpointConnectionError, and a generic Exception fallback);
reference the existing upload_kwargs, progress_callback, s3.upload_file,
BUCKET_NAME, file_path and file_name to locate the call, log/print a clear error
message including the exception details, and exit with a non-zero status (or
re-raise) so CI sees the failure instead of a raw traceback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01f4a27f-5053-4881-8818-44fba31093e4
📒 Files selected for processing (2)
.github/workflows/release.ymlscripts/cloudflare-r2-upload.py
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…1073) Upload nightly release to Cloudflare R2 and announce to early access telegram channel; remove obsolete Greengeeks ssh setup
Upload nightly release to Cloudflare R2 and announce to private telegram channel
Remove obsolete Greengeeks ssh setup