feat: use cloudflare r2 for storing slide pdfs
#1477
Conversation
|
Warning Rate limit exceeded@danielroe has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 50 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes in the pull request involve significant updates to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Handler as Slides Event Handler
participant S3 as Cloudflare R2
participant Config as Runtime Config
Client->>Handler: Request signed URL
Handler->>Config: Retrieve configuration settings
Handler->>S3: Check if object exists
alt Object exists
S3-->>Handler: Return object details
else Object does not exist
Handler->>S3: Upload file to R2
end
Handler-->>Client: Return signed URL
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Deploying danielroe with
|
| Latest commit: |
9ba2a24
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b406aae0.danielroe.pages.dev |
| Branch Preview URL: | https://build-cf-module.danielroe.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
nuxt.config.ts (2)
Line range hint
307-316: Consider security implications of disabled COEPWhilst the CSP changes for GitHub avatars are necessary, disabling the Cross-Origin Embedder Policy might have security implications. Consider:
- Using
requireCorpinstead of completely disabling COEP- Adding a more restrictive CSP by specifying exact domains for image sources
Line range hint
251-279: Remove duplicate domain entriesThe domains list contains a duplicate entry for 'conf.vuejs.de'. Consider:
- Removing the duplicate
- Adding a comment to indicate which domains are temporary (event-specific)
- Sorting the domains alphabetically for better maintenance
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
nuxt.config.ts(1 hunks)
🔇 Additional comments (3)
nuxt.config.ts (3)
Line range hint 297-300: LGTM: Well-configured analytics setup
Using a custom subdomain for Plausible analytics is a good practice for better privacy and tracking protection.
Line range hint 142-149: Verify stability of experimental features
Enabling multiple experimental features could affect production stability. Consider:
- Documenting why each experimental feature is needed
- Adding feature flags to disable them quickly if issues arise
Let's check for any known issues with these features:
#!/bin/bash
# Search for any reported issues with the experimental features
rg -i "cookieStore|headNext|viewTransition|typedPages" .github/issues150-150: Verify compatibility date implications
The compatibility date has been set quite far in the future. This might introduce breaking changes or deprecations that could affect the application.
Let's check for any compatibility-related changes in the repository:
✅ Verification successful
Compatibility date setting is appropriate
The compatibility date of '2024-09-19' aligns with the project's modern dependencies, including Nuxt v3.14.159 and Vue 3.5.12. The configuration also shows a compatibility version of 4, which is the current standard. This forward-dated setting is actually beneficial for ensuring the application maintains consistent behaviour with current features.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for compatibility-related changes or issues
rg -i "compatibility|breaking.?changes?" README.md
Length of output: 126
Script:
#!/bin/bash
# Let's check the package.json for Nuxt version and dependencies
cat package.json
# Also check for any migration guides or compatibility notes in docs
fd -t f -e md . | xargs rg -i "compatibility|breaking.?changes?"
# Check for Nuxt configuration patterns
rg -i "compatibility" --type ts
Length of output: 3689
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected] |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (3)
modules/runtime/server/slides.ts (1)
10-18: Consider making bucket configuration more flexible.The S3 client configuration could benefit from making the bucket name configurable through runtime config.
Consider moving the bucket name to the runtime config:
const s3Client = new S3Client({ endpoint: config.cloudflare.s3Url, region: 'auto', credentials: { accessKeyId: config.cloudflare.r2TokenId, secretAccessKey: config.cloudflare.r2TokenKey, }, }) +const bucket = config.cloudflare.bucket || 'slides'nuxt.config.ts (1)
126-126: Verify cache duration for thumbnailsThe cache duration of one year for thumbnails is quite aggressive. Ensure you have a strategy in place for cache invalidation if thumbnails need updating.
Consider implementing a cache-busting mechanism, such as including a version or hash in the thumbnail URLs.
modules/slides.ts (1)
2-2: Remove unused import 'useNuxt'The
useNuxtfunction is imported from'nuxt/kit'but is not utilised in the code. Removing unused imports helps to keep the codebase clean and maintainable.Apply this diff to remove the unused import:
-import { addServerHandler, createResolver, defineNuxtModule, useNuxt, useRuntimeConfig } from 'nuxt/kit' +import { addServerHandler, createResolver, defineNuxtModule, useRuntimeConfig } from 'nuxt/kit'🧰 Tools
🪛 GitHub Check: test
[failure] 2-2:
'useNuxt' is defined but never used. Allowed unused vars must match /^_/u
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
package.jsonis excluded by!package.json,!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (4)
.gitignore(2 hunks)modules/runtime/server/slides.ts(1 hunks)modules/slides.ts(1 hunks)nuxt.config.ts(3 hunks)
🧰 Additional context used
🪛 GitHub Check: test
modules/slides.ts
[failure] 2-2:
'useNuxt' is defined but never used. Allowed unused vars must match /^_/u
🔇 Additional comments (6)
modules/runtime/server/slides.ts (1)
1-3: LGTM! Proper import declarations.
The imports are well-structured, using the recommended node: prefix for native modules and importing specific functions from AWS SDK packages.
.gitignore (2)
61-61: Good addition for security!
Adding .env.production to .gitignore is crucial for preventing production environment variables from being accidentally committed.
100-101: Appropriate exclusion for Wrangler files
The addition of .wrangler aligns with the PR's objective of integrating the Cloudflare module preset. This ensures that local Wrangler development settings and caches aren't tracked in version control.
nuxt.config.ts (2)
155-155: Verify compatibility date requirements
The compatibility date is set to '2024-09-19', which is a future date. Please confirm this is intentional and verify the specific Cloudflare Workers features that require this future compatibility date.
Let's check the Cloudflare Workers documentation for this compatibility date:
#!/bin/bash
# Search for any Cloudflare Workers-related configurations
rg -g '!node_modules' 'workers.*compatibility'124-124: Review CORS configuration for API endpoint
The /api/hi endpoint has CORS enabled. Consider restricting the CORS configuration to specific origins rather than allowing all origins.
Let's check for any existing CORS configurations:
#!/bin/bash
# Search for CORS-related configurations
rg -g '!node_modules' -A 5 'cors:.*true' modules/slides.ts (1)
86-86: Ensure 'config.accountId' is defined in runtime configuration
In the code snippet for obtaining a Cloudflare R2 token, config.accountId is referenced but may not be defined in your runtime configuration. Please verify that config.accountId is set within useRuntimeConfig() to prevent potential issues.
Would you like assistance in updating the runtime configuration to include accountId?
danielroe
changed the title
wrangler.toml for cloudflare_module presetr2 for storing slide pdfs
Summary by CodeRabbit
New Features
Improvements
Configuration Updates
cookieStore,headNext, andviewTransition..gitignoreentries to exclude additional development files and directories.