chore(samples): build apk and upload to aws #2266
Conversation
renefloor
changed the title
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
""" WalkthroughThe Android GitHub Actions workflow was updated to replace Firebase distribution with an AWS S3 upload using a new Fastlane lane. The APK output filename was standardized, and the Fastlane configuration was extended with an AWS S3 plugin and a new lane to build and upload the APK to S3. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHubActions as GitHub Actions Workflow
participant Fastlane as Fastlane
participant AWS as AWS S3
GitHubActions->>Fastlane: Run `distribute_to_s3` lane with AWS creds
Fastlane->>Fastlane: Build APK (fixed name)
Fastlane->>AWS: Upload APK to S3 bucket with encryption
AWS-->>Fastlane: Upload confirmation
Fastlane-->>GitHubActions: Lane completion
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/distribute_external.yml (3)
77-79: Explicitly specify--releaseand cache pub dependencies for faster buildsBy default,
flutter build apkproduces a release build, but adding--releasemakes it explicit. Also, caching pub dependencies can significantly speed up CI runs.Example diffs:
- - name: Build - run: flutter build apk + - name: Build + run: flutter build apk --release working-directory: sample_app+ - name: Cache Flutter Pub dependencies + uses: actions/cache@v3 + with: + path: ~/.pub-cache + key: ${{ runner.os }}-pub-${{ hashFiles('**/pubspec.lock') }}
81-85: Make the artifact name dynamic and set retentionThe static artifact name (
android-stream-chat-v1) can become confusing over time. Incorporate the release tag or build number, and addretention-daysto automatically prune old artifacts.Example diff:
- - name: Upload Artifact - uses: actions/upload-artifact@v4 - with: - name: android-stream-chat-v1 - path: sample_app/build/app/outputs/apk/release/app-release.apk + - name: Upload Artifact + uses: actions/upload-artifact@v4 + with: + name: android-stream-chat-${{ github.ref_name }} + path: sample_app/build/app/outputs/apk/release/app-release.apk + retention-days: 30
94-98: Simplify the S3 upload step by removing the intermediate copyThe
cpcommand and intermediate file are unnecessary. Upload directly from the build output to S3 in one go.Suggested diff:
- - name: Upload to S3 - run: | - cp sample_app/build/app/outputs/apk/release/app-release.apk flutter-sample-app.apk - aws s3 cp flutter-sample-app.apk s3://${{ secrets.AWS_S3_BUCKET }} --sse AES256 + - name: Upload to S3 + run: aws s3 cp sample_app/build/app/outputs/apk/release/app-release.apk s3://${{ secrets.AWS_S3_BUCKET }}/flutter-sample-app.apk --sse AES256
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/distribute_external.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/distribute_external.yml
88-88: the runner of "aws-actions/configure-aws-credentials@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
sample_app/android/app/build.gradle (1)
84-88: APK naming standardization looks good, consider variant-specific naming for robustness.The fixed APK naming facilitates consistent artifact handling for the AWS S3 upload process. However, consider the following potential improvements:
Variant conflicts: The current implementation applies the same filename to all variants (debug, release, etc.), which could cause file conflicts if multiple variants are built simultaneously.
Build type awareness: Consider incorporating build type in the filename for better traceability.
If you want to maintain variant-specific naming while keeping consistency for release builds, consider this approach:
applicationVariants.all { variant -> variant.outputs.all { - outputFileName = "flutter-sample-app.apk" + if (variant.buildType.name == "release") { + outputFileName = "flutter-sample-app.apk" + } else { + outputFileName = "flutter-sample-app-${variant.buildType.name}.apk" + } } }sample_app/android/fastlane/Fastfile (1)
55-76: Well-implemented S3 distribution lane with room for minor improvements.The new
distribute_to_s3lane is well-structured with proper validation and security considerations. The implementation correctly:
- Reuses the existing
build_apklane for modularity- Validates bucket and path configuration
- Uses secure environment variable handling
- Applies server-side encryption
Consider these enhancements for robustness:
- More flexible bucket parsing: The current split logic assumes a simple bucket/path structure.
- bucket_name, path = bucket_with_path.split('/', 2) + parts = bucket_with_path.split('/', 2) + bucket_name = parts[0] + path = parts[1..-1].join('/') if parts.length > 1
Build versioning consistency: Unlike the Firebase lane, this doesn't handle build numbering. Consider if versioning is needed for S3 uploads.
Upload verification: Consider adding a verification step to confirm successful upload.
+ # Verify upload success + UI.success("Successfully uploaded APK to s3://#{bucket_name}/#{path}/flutter-sample-app.apk")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/distribute_external.yml(1 hunks)sample_app/android/app/build.gradle(1 hunks)sample_app/android/fastlane/Fastfile(1 hunks)sample_app/android/fastlane/Pluginfile(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- sample_app/android/fastlane/Pluginfile
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/distribute_external.yml
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: android
- GitHub Check: stream_chat_flutter
- GitHub Check: stream_chat_localizations
🔇 Additional comments (1)
sample_app/android/fastlane/Fastfile (1)
48-48: APK path update aligns correctly with build.gradle changes.The updated artifact path correctly reflects the new standardized APK naming convention configured in
sample_app/android/app/build.gradle.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
sample_app/android/fastlane/Fastfile (3)
65-65: Enhance path validation for better robustness.The current validation only checks for
nilor empty strings. Consider trimming whitespace and validating the path format more thoroughly.- UI.user_error!("AWS_S3_BUCKET must include path (e.g., 'bucket-name/downloads')") if path.nil? || path.empty? + path = path&.strip + UI.user_error!("AWS_S3_BUCKET must include path (e.g., 'bucket-name/downloads')") if path.nil? || path.empty?
58-58: Extract APK path to avoid code duplication.The APK file path is hardcoded in both the Firebase and S3 lanes. Consider extracting this to a method or constant to improve maintainability.
Add a helper method at the top of the platform block:
platform :android do app_id = "1:674907137625:android:f4033353c54985b0d7f348" # The Firebase app ID + + def apk_output_path + "#{root_path}/build/app/outputs/apk/release/flutter-sample-app.apk" + endThen update both lanes to use this method:
- android_artifact_path: "#{root_path}/build/app/outputs/apk/release/flutter-sample-app.apk", + android_artifact_path: apk_output_path,- files: ["#{root_path}/build/app/outputs/apk/release/flutter-sample-app.apk"] + files: [apk_output_path]Also applies to: 74-74
67-75: Consider adding error handling for AWS operations.The S3 upload operation could fail due to network issues, invalid credentials, or insufficient permissions. Consider wrapping the
aws_s3call in error handling to provide clearer feedback.- aws_s3( - access_key: ENV.fetch("AWS_ACCESS_KEY_ID"), - secret_access_key: ENV.fetch("AWS_SECRET_ACCESS_KEY"), - region: ENV.fetch("AWS_REGION", "us-east-1"), - bucket: bucket_name, - path: path, - server_side_encryption: "AES256", - files: ["#{root_path}/build/app/outputs/apk/release/flutter-sample-app.apk"] - ) + begin + aws_s3( + access_key: ENV.fetch("AWS_ACCESS_KEY_ID"), + secret_access_key: ENV.fetch("AWS_SECRET_ACCESS_KEY"), + region: ENV.fetch("AWS_REGION", "us-east-1"), + bucket: bucket_name, + path: path, + server_side_encryption: "AES256", + files: ["#{root_path}/build/app/outputs/apk/release/flutter-sample-app.apk"] + ) + UI.success("APK successfully uploaded to S3: s3://#{bucket_name}/#{path}") + rescue => e + UI.user_error!("Failed to upload APK to S3: #{e.message}") + end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sample_app/android/fastlane/Fastfile(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: android
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat_flutter
- GitHub Check: stream_chat_persistence
🔇 Additional comments (2)
sample_app/android/fastlane/Fastfile (2)
48-48: LGTM! APK path update aligns with new naming convention.The updated path correctly reflects the new standardized APK filename and output location.
55-76: Well-implemented S3 distribution lane with good security practices.The new
distribute_to_s3lane is well-structured and includes:
- Proper environment variable usage with
ENV.fetch- Bucket/path parsing and validation
- AES256 server-side encryption for security
- Clear documentation and usage examples
The implementation follows Fastlane best practices and provides a clean replacement for Firebase distribution.
xsahil03x
force-pushed
the
chore/publish-sample-to-aws
branch
from
210833b to
331bf82
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sample_app/android/fastlane/Fastfile (1)
55-108: Consider reducing verbose logging and improving error handling.The new S3 distribution lane includes comprehensive logging for debugging APK file locations, which is helpful for troubleshooting but may be excessive for production use.
Consider the following improvements:
desc "Build and distribute app to S3" # Usage: bundle exec fastlane android distribute_to_s3 lane :distribute_to_s3 do |options| build_apk - # Add logging to verify APK file availability - UI.message("🔍 Checking APK build outputs...") - - # List contents of the build directory - build_dir = "#{root_path}/build/app/outputs" - if Dir.exist?(build_dir) - UI.message("📁 Contents of #{build_dir}:") - Dir.glob("#{build_dir}/**/*").each do |file| - UI.message(" - #{file}") - end - else - UI.error("❌ Build directory does not exist: #{build_dir}") - end - - # Check specific APK locations - expected_apk = "#{root_path}/build/app/outputs/apk/release/flutter-sample-app.apk" - actual_apk = "#{root_path}/build/app/outputs/flutter-apk/app-release.apk" - - UI.message("🎯 Checking expected APK location: #{expected_apk}") - if File.exist?(expected_apk) - UI.success("✅ Expected APK found!") - file_size = File.size(expected_apk) / (1024 * 1024) # Size in MB - UI.message("📦 APK size: #{file_size.round(1)} MB") - else - UI.error("❌ Expected APK not found") - end - - UI.message("🎯 Checking actual APK location: #{actual_apk}") - if File.exist?(actual_apk) - UI.success("✅ Actual APK found!") - file_size = File.size(actual_apk) / (1024 * 1024) # Size in MB - UI.message("📦 APK size: #{file_size.round(1)} MB") - else - UI.error("❌ Actual APK not found") - end - - # Find all APK files - UI.message("🔎 Searching for all APK files in build directory...") - apk_files = Dir.glob("#{build_dir}/**/*.apk") - if apk_files.any? - UI.message("📱 Found APK files:") - apk_files.each do |apk| - file_size = File.size(apk) / (1024 * 1024) # Size in MB - UI.message(" - #{apk} (#{file_size.round(1)} MB)") - end - else - UI.error("❌ No APK files found in build directory!") - end + # Verify APK exists before upload + apk_path = "#{root_path}/build/app/outputs/apk/release/flutter-sample-app.apk" + unless File.exist?(apk_path) + UI.user_error!("APK file not found at: #{apk_path}") + end + + file_size = File.size(apk_path) / (1024 * 1024) + UI.message("📦 Uploading APK (#{file_size.round(1)} MB) to S3...")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sample_app/android/fastlane/Fastfile(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: android
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat_flutter
🔇 Additional comments (1)
sample_app/android/fastlane/Fastfile (1)
48-48: LGTM! APK path standardization looks good.The APK path has been updated to align with the new naming convention. This change maintains consistency across the Firebase distribution workflow.
xsahil03x
force-pushed
the
chore/publish-sample-to-aws
branch
from
eecb5cd to
fb8822b
Compare
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
Submit a pull request
Linear: fixes FLU-150
Github Issue: #
CLA
Description of the pull request
This copies the publishing code from flutter-samples so we publish the apk to aws again and have it downloadable from the website.
Summary by CodeRabbit