-
-
Notifications
You must be signed in to change notification settings - Fork 701
Docs: Batch Trigger upgrades #1505
New issue
Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “No 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? No Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces various modifications across multiple documentation files related to the Trigger.dev platform. Key changes include the reformatting of code snippets for readability, updates to the documentation on idempotency, and the addition of new sections detailing task limits and real-time API functionalities. The changes also involve restructuring existing documents to enhance clarity and provide new examples for task triggering and management. Overall, the updates aim to improve the documentation's usability without altering the underlying functionality. Changes
Possibly related PRs
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 (
|
a0f8f80
to
75ddd4b
Compare
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.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (13)
docs/realtime/subscribe-to-batch.mdx (3)
11-17
: Consider adding error handling and cleanup example.While the example demonstrates the basic usage, it would be helpful to show error handling and proper cleanup. This is especially important for long-running subscriptions.
Consider expanding the example:
```ts Example import { runs } from "@trigger.dev/sdk/v3"; -for await (const run of runs.subscribeToBatch("batch_1234")) { - console.log(run); -} +try { + for await (const run of runs.subscribeToBatch("batch_1234")) { + console.log(run); + // Break the loop when certain conditions are met + if (someCondition) break; + } +} catch (error) { + console.error("Subscription error:", error); +} finally { + // Cleanup code if needed +}
21-21
: Fix grammar issues in the description.There are two grammar issues in this line:
- Double articles: "yields the a run object"
- Incorrect possessive form: "it's" should be "its"
-This function subscribes to all changes for runs in a batch. It returns an async iterator that yields the a run object whenever a run in the batch is updated. The iterator does not complete on it's own, you must manually `break` the loop when you want to stop listening for updates. +This function subscribes to all changes for runs in a batch. It returns an async iterator that yields a run object whenever a run in the batch is updated. The iterator does not complete on its own, you must manually `break` the loop when you want to stop listening for updates.🧰 Tools
🪛 LanguageTool
[grammar] ~21-~21: Two determiners in a row. Choose either “the” or “a”.
Context: ...t returns an async iterator that yields the a run object whenever a run in the batch ...(DT_DT)
[uncategorized] ~21-~21: Did you mean “its” (the possessive pronoun)?
Context: ...ated. The iterator does not complete on it's own, you must manuallybreak
the loop...(ITS_PREMIUM)
45-49
: Consider adding examples of common properties.While referencing the RunObject component is good for completeness, it would be helpful to show the most commonly used properties inline.
Consider adding:
The AsyncIterator yields an object with the following properties: +Common properties include: +```ts +{ + id: "run_123", + status: "RUNNING" | "COMPLETED" | "FAILED", + startedAt: "2024-01-01T00:00:00Z", + completedAt: "2024-01-01T00:01:00Z", + // ... see RunObject for full list of properties +} +``` + <RunObject />docs/limits.mdx (2)
60-60
: Add missing comma before 'however'For better readability and correct grammar, add a comma before 'however'.
-Payloads and outputs that exceed 512KB will be offloaded to object storage and a presigned URL will be provided to download the data when calling `runs.retrieve`. You don't need to do anything to handle this in your tasks however, as we will transparently upload/download these during operation. +Payloads and outputs that exceed 512KB will be offloaded to object storage and a presigned URL will be provided to download the data when calling `runs.retrieve`. You don't need to do anything to handle this in your tasks, however, as we will transparently upload/download these during operation.🧰 Tools
🪛 LanguageTool
[uncategorized] ~60-~60: Possible missing comma found.
Context: ...xceed 512KB will be offloaded to object storage and a presigned URL will be provided to...(AI_HYDRA_LEO_MISSING_COMMA)
[formatting] ~60-~60: Consider inserting a comma before ‘however’.
Context: ...d to do anything to handle this in your tasks however, as we will transparently upload/downloa...(HOWEVER_MISSING_COMMA)
49-50
: Consider varying sentence structureTo improve readability, consider rewording to avoid three consecutive sentences starting with "If".
-If you add them [dynamically using code](/management/schedules/create) make sure you add a `deduplicationKey` so you don't add the same schedule to a task multiple times. If you don't your task will get triggered multiple times, it will cost you more, and you will hit the limit. If you're creating schedules for your user you will definitely need to request more schedules from us. +When adding schedules [dynamically using code](/management/schedules/create), make sure to include a `deduplicationKey` to prevent duplicate schedule creation. Without this key, your task will trigger multiple times, increasing costs and potentially hitting limits. For user-specific schedule creation, you'll need to request an increased schedule allocation from our team.🧰 Tools
🪛 LanguageTool
[style] ~49-~49: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... you more, and you will hit the limit. If you're creating schedules for your user...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
docs/idempotency.mdx (3)
Line range hint
18-91
: Code examples are comprehensive and well-documented.The examples effectively demonstrate various idempotency key usage patterns. Consider adding a TypeScript type annotation for the
options
parameter in the examples to help users understand the complete API surface.Add type annotations like this:
-await childTask.trigger({ foo: "bar" }, { idempotencyKey }); +await childTask.trigger({ foo: "bar" }, { idempotencyKey }: { idempotencyKey: string });
97-97
: Add missing comma for better readability.Add a comma after "By default" for proper punctuation.
-By default idempotency keys are stored for 30 days. +By default, idempotency keys are stored for 30 days.
Line range hint
135-149
: Consider adding a stable serialization example.The hash function implementation is secure, but as noted in the comment, stable serialization is crucial. Consider adding an example using a stable serialization library like
json-stable-stringify
.function hash(payload: any): string { const hash = createHash("sha256"); - hash.update(JSON.stringify(payload)); + // Use json-stable-stringify for consistent object key ordering + const stableJson = require('json-stable-stringify'); + hash.update(stableJson(payload)); return hash.digest("hex"); }🧰 Tools
🪛 LanguageTool
[uncategorized] ~96-~96: Did you mean: “By default,”?
Context: ...]); ``` ##idempotencyKeyTTL
option By default idempotency keys are stored for 30 days...(BY_DEFAULT_COMMA)
docs/runs.mdx (3)
203-209
: Consider adding performance guidance for async iteratorsWhile the async iterator example is elegant, it might be helpful to add a note about memory usage and performance implications when dealing with large datasets. Users should be aware that this approach loads all results into memory.
230-231
: Fix grammatical error in the descriptionChange "it's" to "its" as it's being used as a possessive pronoun.
-Fetch a single run by it's ID: +Fetch a single run by its ID:🧰 Tools
🪛 LanguageTool
[uncategorized] ~230-~230: Did you mean “its” (the possessive pronoun)?
Context: ... runs.retrieve() Fetch a single run by it's ID: ```ts import { runs } from "@trigg...(ITS_PREMIUM)
298-303
: Consider adding error handling examplesThe real-time subscription example could benefit from showing how to handle connection errors or disconnects. This would help users implement more robust solutions.
Example addition:
try { for await (const run of runs.subscribeToRun(runId)) { console.log(run); } } catch (error) { if (error instanceof ConnectionError) { // Handle connection errors console.error("Lost connection to real-time updates"); } // Handle other errors }docs/triggering.mdx (2)
17-17
: Fix grammatical error in heading textChange "from inside a another task" to "from inside another task".
-Trigger tasks **from inside a another task**: +Trigger tasks **from inside another task**:🧰 Tools
🪛 LanguageTool
[misspelling] ~17-~17: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...) | Trigger tasks from inside a another task: | Function ...(EN_A_VS_AN)
832-836
: Consider enhancing the visibility of version-specific warningThis is a critical warning about
idempotencyKey
not being available in v3.3.0+ for specific functions. Consider making it more prominent, perhaps by:
- Moving it closer to the relevant function documentation
- Adding it to the function-specific sections as well
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
docs/guides/examples/scrape-hacker-news.mdx
(4 hunks)docs/idempotency.mdx
(7 hunks)docs/limits.mdx
(5 hunks)docs/mint.json
(1 hunks)docs/realtime/subscribe-to-batch.mdx
(1 hunks)docs/runs.mdx
(5 hunks)docs/triggering.mdx
(8 hunks)docs/v3-openapi.yaml
(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/guides/examples/scrape-hacker-news.mdx
🧰 Additional context used
🪛 LanguageTool
docs/idempotency.mdx
[uncategorized] ~96-~96: Did you mean: “By default,”?
Context: ...]); ``` ## idempotencyKeyTTL
option By default idempotency keys are stored for 30 days...
(BY_DEFAULT_COMMA)
docs/limits.mdx
[style] ~49-~49: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... you more, and you will hit the limit. If you're creating schedules for your user...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~60-~60: Possible missing comma found.
Context: ...xceed 512KB will be offloaded to object storage and a presigned URL will be provided to...
(AI_HYDRA_LEO_MISSING_COMMA)
[formatting] ~60-~60: Consider inserting a comma before ‘however’.
Context: ...d to do anything to handle this in your tasks however, as we will transparently upload/downloa...
(HOWEVER_MISSING_COMMA)
docs/realtime/subscribe-to-batch.mdx
[grammar] ~21-~21: Two determiners in a row. Choose either “the” or “a”.
Context: ...t returns an async iterator that yields the a run object whenever a run in the batch ...
(DT_DT)
[uncategorized] ~21-~21: Did you mean “its” (the possessive pronoun)?
Context: ...ated. The iterator does not complete on it's own, you must manually break
the loop...
(ITS_PREMIUM)
docs/runs.mdx
[uncategorized] ~230-~230: Did you mean “its” (the possessive pronoun)?
Context: ... runs.retrieve() Fetch a single run by it's ID: ```ts import { runs } from "@trigg...
(ITS_PREMIUM)
docs/triggering.mdx
[misspelling] ~17-~17: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...) | Trigger tasks from inside a another task: | Function ...
(EN_A_VS_AN)
[style] ~97-~97: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...n, and any options you specify, without needing to import the task. ```ts Your backend im...
(REP_NEED_TO_VB)
[style] ~645-~645: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...when you have a static set of tasks you want to trigger: ```ts /trigger/batch.ts impor...
(REP_WANT_TO_VB)
[style] ~701-~701: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...# Triggering from your frontend If you want to trigger a task directly from a frontend...
(REP_WANT_TO_VB)
🔇 Additional comments (11)
docs/realtime/subscribe-to-batch.mdx (1)
1-5
: LGTM!
The frontmatter is well-structured and provides clear, concise information about the functionality.
docs/limits.mdx (3)
6-6
: LGTM!
The import statement is correctly formatted and follows React component naming conventions.
10-14
: LGTM!
The table formatting changes improve consistency and readability across all sections.
Also applies to: 23-23, 33-33, 40-44, 70-74, 119-123, 133-137
52-58
: LGTM!
The new sections clearly document important limits for task payloads, outputs, and batch sizes. The information is well-structured and provides crucial details for users.
Also applies to: 62-64
docs/idempotency.mdx (2)
99-122
: Verify the default TTL value and consider adding edge cases.
The example effectively demonstrates TTL usage. Consider adding:
- Maximum allowed TTL value
- Behavior when an invalid TTL format is provided
- Error handling examples for expired keys
✅ Verification successful
Let me search for more specific information about TTL validation and default values.
Based on the search results, I can now provide a comprehensive response about the TTL behavior:
TTL validation and default behavior is well-implemented, example is accurate
The example in the documentation accurately represents the TTL behavior. From the codebase:
- TTL format is validated using regex pattern
^(\d+)([smhd])$
(numbers followed by s/m/h/d units) - If TTL is not specified, it defaults to 30 days
- Invalid TTL formats are gracefully handled by returning undefined
- The implementation in
resolveIdempotencyKeyTTL
properly validates and processes the TTL values
The example effectively demonstrates the key aspects:
- Shows correct TTL format ("60s")
- Demonstrates expiration behavior
- Illustrates retry scenario with expired key
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for TTL-related configuration or validation
rg -A 5 "idempotencyKeyTTL" --type ts
Length of output: 12586
Script:
#!/bin/bash
# Search for TTL resolution and validation logic
rg -A 10 "resolveIdempotencyKeyTTL" --type ts
# Search for any duration parsing or validation
rg -A 5 "parseDuration|validateDuration" --type ts
Length of output: 6803
8-12
: Verify version information and consider adding migration guidance.
The warning about the idempotencyKey
limitation is clear and important. Consider enhancing it with:
- The exact version where this bug was introduced
- Temporary workaround suggestions for users who need both idempotency and wait functionality
docs/mint.json (1)
220-221
: LGTM! Verify the referenced documentation file exists.
The addition of the "subscribe-to-batch" page is properly placed within the "Realtime API" section, maintaining a logical flow with related functionality.
Let's verify that the referenced documentation file exists:
✅ Verification successful
Documentation file exists as expected
The referenced documentation file docs/realtime/subscribe-to-batch.mdx
exists in the correct location, confirming proper documentation structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the referenced documentation file exists
# Expected: File should exist since it's referenced in the navigation
fd "subscribe-to-batch.mdx" docs/realtime/
Length of output: 79
docs/runs.mdx (1)
101-102
: LGTM! Code examples are clear and demonstrate proper async/await usage.
Also applies to: 115-115
docs/v3-openapi.yaml (3)
Line range hint 809-1141
: Well-structured environment variables API endpoints
The environment variables endpoints are well-designed with:
- Clear REST conventions
- Proper authentication
- Comprehensive examples
- Appropriate error handling
Line range hint 1142-1520
: Well-implemented batch triggering functionality
The batch triggering endpoint is properly implemented with:
- Clear documentation
- Appropriate request/response schemas
- Validation limits
- Multiple usage examples (SDK, curl, Python)
Line range hint 1521-2521
: Well-defined schema definitions
The schema definitions are comprehensive and include:
- Clear property descriptions
- Appropriate validation rules
- Helpful examples
- Proper type definitions
To generate a public access token, use the `auth.createPublicToken` function: | ||
|
||
```ts | ||
import { auth } from "@trigger.dev/sdk/v3"; | ||
|
||
// Somewhere in your backend code | ||
const publicToken = await auth.createPublicToken({ | ||
scopes: { | ||
read: { | ||
batch: ["batch_1234"], | ||
}, | ||
}, | ||
}); | ||
``` |
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.
🛠️ Refactor suggestion
Add security warning about token handling.
While the authentication example is clear, it would be beneficial to add a security warning about token exposure.
Add a security note after the code example:
});
+>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
`````suggestion
To generate a public access token, use the `auth.createPublicToken` function:
description: Subscribes to all changes for runs in a batch. | ||
--- | ||
|
||
import RunObject from "/snippets/realtime/run-object.mdx"; |
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.
💡 Codebase verification
Import path should be updated to match the correct location
The run-object.mdx
snippet exists but at a different path: docs/snippets/realtime/run-object.mdx
. The current import path /snippets/realtime/run-object.mdx
should be updated to match the actual location.
🔗 Analysis chain
Verify the RunObject snippet exists.
The import suggests a modular documentation structure. Let's verify the snippet file exists.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the RunObject snippet exists
fd "run-object.mdx" snippets/realtime
Length of output: 142
Script:
#!/bin/bash
# Let's first check if the file exists in the repository
fd "run-object.mdx"
Length of output: 57
Script:
#!/bin/bash
# Let's also check the actual directory structure under docs
fd . docs --type d
Length of output: 470
Script:
#!/bin/bash
# Let's examine the content of the docs/realtime directory
ls -la docs/realtime
Length of output: 560
export const childTask2 = task({ | ||
id: "child-task-2", | ||
run: async (payload: { bar: number }) => { | ||
return bar + 1; | ||
}, | ||
}); |
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.
Fix undefined variable reference in code example
The code example references an undefined variable bar
. It should use payload.bar
instead.
export const childTask2 = task({
id: "child-task-2",
run: async (payload: { bar: number }) => {
- return bar + 1;
+ return payload.bar + 1;
},
});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const childTask2 = task({ | |
id: "child-task-2", | |
run: async (payload: { bar: number }) => { | |
return bar + 1; | |
}, | |
}); | |
export const childTask2 = task({ | |
id: "child-task-2", | |
run: async (payload: { bar: number }) => { | |
return payload.bar + 1; | |
}, | |
}); |
@@ -827,4 +1056,4 @@ export const myTask = task({ | |||
|
|||
### Batch Triggering | |||
|
|||
When using `batchTrigger` or `batchTriggerAndWait`, the total size of all payloads cannot exceed 10MB. This means if you are doing a batch of 100 runs, each payload should be less than 100KB. | |||
When using triggering a batch, the total size of all payloads cannot exceed 1MB. This means if you are doing a batch of 100 runs, each payload should be less than 100KB. The max batch size is 500 runs. |
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.
💡 Codebase verification
Documentation contains conflicting payload size limits
The documentation has contradictory information about payload size limits:
- At one location it states: "We currently have a hard limit on task payloads above 10MB"
- At line 1059 it states: "When using triggering a batch, the total size of all payloads cannot exceed 1MB"
Please clarify:
- Whether the 10MB limit applies to individual task payloads
- Whether the 1MB limit is specifically for batch operations
- If both limits are correct, make the distinction between these scenarios more explicit
🔗 Analysis chain
Clarify payload size limits
There's an inconsistency in the documentation regarding payload limits. Earlier in the document, it mentions a 10MB limit for individual payloads, but here it states a 1MB limit for batch payloads. Please clarify if these are separate limits for different scenarios.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other mentions of payload limits in the documentation
rg -i "payload.*limit|limit.*payload" --type mdx
Length of output: 78
Script:
#!/bin/bash
# Search for mentions of payload limits in documentation files, including markdown and mdx
rg -i "payload.*limit|limit.*payload" docs/
Length of output: 187
@@ -753,7 +753,7 @@ paths: | |||
import { runs, configure } from "@trigger.dev/sdk/v3"; | |||
|
|||
configure({ | |||
secretKey: "tr_pat_1234" // always use an environment variable for this | |||
accessToken: "tr_pat_1234" // always use an environment variable for this |
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.
Use environment variables for access tokens
The code examples show hardcoded access tokens which is a security risk. Always use environment variables for sensitive credentials.
- accessToken: "tr_pat_1234" // always use an environment variable for this
+ accessToken: process.env.TRIGGER_ACCESS_TOKEN
Also applies to: 784-784
75ddd4b
to
011ecc5
Compare
Summary by CodeRabbit
Release Notes
New Features
idempotencyKeyTTL
option for task triggering.runs.subscribeToBatch
function to subscribe to batch updates.Documentation Updates
Bug Fixes
idempotencyKey
usage in specific contexts.