Skip to content

Fix default machine preset in config not being used #1321

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

Merged
merged 2 commits into from
Sep 18, 2024

Conversation

ericallam
Copy link
Member

@ericallam ericallam commented Sep 18, 2024

The default machine preset set in the trigger.config.ts file was not being applied to tasks that didn't already have a machine preset set.

Summary by CodeRabbit

  • New Features

    • Introduced a default machine preset configuration for easier setup in "trigger.dev" settings.
    • Added a new environment variable TRIGGER_INDEXING to influence indexing behavior.
    • Enhanced task configuration with a specific machine preset in task execution.
  • Bug Fixes

    • Improved error handling with detailed messages and stack traces for task file imports and internal errors.
  • Chores

    • Updated machine type in project configuration from "small-2x" to "medium-1x" for better performance.

Copy link

changeset-bot bot commented Sep 18, 2024

🦋 Changeset detected

Latest commit: 7b2fba1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
trigger.dev Patch
@trigger.dev/build Patch
@trigger.dev/core Patch
@trigger.dev/sdk Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

coderabbitai bot commented Sep 18, 2024

Walkthrough

The changes introduce several enhancements across various files, focusing on configuration management and error handling. Key updates include the addition of a default machine preset in the "trigger.dev" settings, improved error messages and stack traces for task file imports, and modifications to the configuration handling in deployment processes. Additionally, new properties are added to error objects and task configurations, enhancing the robustness of the system and providing clearer insights during debugging.

Changes

Files Change Summary
.changeset/new-yaks-fail.md Added default machine preset configuration in "trigger.dev" settings.
.changeset/quick-bulldogs-float.md Enhanced error handling for task file imports with additional error messages and stack traces.
packages/cli-v3/src/entryPoints/deploy-index-worker.ts Modified bootstrap function to return a config object for task manifests.
packages/cli-v3/src/entryPoints/deploy-run-worker.ts Improved error messages for COULD_NOT_FIND_TASK and COULD_NOT_IMPORT_TASK errors.
packages/cli-v3/src/entryPoints/dev-run-worker.ts Added message and stackTrace properties to the error object in ZodIpcConnection.
packages/cli-v3/src/indexing/indexWorkerManifest.ts Introduced a new environment variable TRIGGER_INDEXING set to "1".
references/v3-catalog/src/trigger/simple.ts Added machine property with preset value "small-1x" to fetchPostTask function.
references/v3-catalog/trigger.config.ts Updated machine property from "small-2x" to "medium-1x" in project configuration.

Possibly related PRs

  • v3: self-hosting support for latest packages #1319: The changes in this PR include enhancements to the configuration handling for self-hosted deployments, which may relate to the default machine preset configuration introduced in the main PR, as both involve improving setup processes and configurations.

Poem

In the meadow where bunnies play,
New presets and configs brighten the day.
Errors now clearer, tasks run with ease,
With each little change, we hop and we please!
So let’s celebrate with a joyful cheer,
For smoother setups, we hold dear! 🐰✨


Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d8006e1 and 7b2fba1.

Files selected for processing (8)
  • .changeset/new-yaks-fail.md (1 hunks)
  • .changeset/quick-bulldogs-float.md (1 hunks)
  • packages/cli-v3/src/entryPoints/deploy-index-worker.ts (1 hunks)
  • packages/cli-v3/src/entryPoints/deploy-run-worker.ts (2 hunks)
  • packages/cli-v3/src/entryPoints/dev-run-worker.ts (1 hunks)
  • packages/cli-v3/src/indexing/indexWorkerManifest.ts (1 hunks)
  • references/v3-catalog/src/trigger/simple.ts (1 hunks)
  • references/v3-catalog/trigger.config.ts (1 hunks)
Additional comments not posted (10)
.changeset/new-yaks-fail.md (1)

1-5: LGTM!

The changeset file follows the standard format and provides a clear description of the change. The AI-generated summary also provides additional context about the purpose and impact of the change.

Applying a default machine preset in the configuration can indeed streamline the setup process, enhance consistency, and reduce the likelihood of configuration errors. This change aligns with the goal of providing a more robust and user-friendly setup experience.

.changeset/quick-bulldogs-float.md (1)

1-5: LGTM!

The changeset file provides a clear and concise description of the changes. The metadata and description sections are properly formatted and convey the necessary information.

The changes described in this file are focused on improving error handling and debugging for task file imports, which is a valuable enhancement for developers. The additional error message and stack trace will provide clearer insights into failures, potentially leading to faster issue resolution.

Overall, the changeset file looks good and the changes it describes are beneficial to the project.

references/v3-catalog/trigger.config.ts (1)

16-16: LGTM! The machine type upgrade should improve performance.

The change from "small-2x" to "medium-1x" for the machine property is a reasonable upgrade that aligns with the PR objectives. This modification suggests an intentional enhancement to the execution environment, likely to improve performance or increase resource availability for the project.

Given that the change is isolated to the machine property and does not introduce any breaking changes or compatibility issues, it can be safely merged.

packages/cli-v3/src/indexing/indexWorkerManifest.ts (1)

52-52: Clarify the purpose of TRIGGER_INDEXING environment variable.

The addition of the TRIGGER_INDEXING environment variable with value "1" suggests it is being used as a flag to trigger the indexing process in the child process spawned by the fork function.

However, it is unclear from the given context:

  1. How is this environment variable being used in the indexWorkerPath script?
  2. How does it relate to the issue of default machine preset not being applied, as described in the PR objective?

Please provide more details on the purpose of TRIGGER_INDEXING and how it addresses the problem statement. If it is unrelated to the machine preset issue, consider removing it from this PR.

packages/cli-v3/src/entryPoints/deploy-index-worker.ts (2)

98-98: LGTM!

The destructuring of the config object from the return value of bootstrap is correct and consistent with the changes made to the bootstrap function.


100-116: Verify the impact of applying the config.machine preset to tasks.

The logic for applying the config.machine preset to tasks that don't have a machine preset defined is correct and consistent with the AI-generated summary.

Please ensure that the application of the config.machine preset to tasks does not have any unintended consequences on the task processing.

Run the following script to verify the impact of the changes:

Verification successful

Verified: config.machine preset application is localized and correct

The impact of applying the config.machine preset to tasks is limited to the file packages/cli-v3/src/entryPoints/deploy-index-worker.ts. The implementation matches the original code snippet we reviewed:

  1. It checks if config.machine is a string.
  2. It applies the config.machine preset only to tasks that don't have a preset defined.

No other occurrences of this logic were found in the codebase, which reduces the potential for unintended consequences. The changes appear to be correct and their impact is well-contained.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the impact of applying the `config.machine` preset to tasks.

# Test 1: Search for tasks that don't have a machine preset defined. Expect: No occurrences.
rg --type typescript -g '!node_modules' -g '!dist' $'tasks\\.(map|forEach)\\(\\([^\\)]+\\)\\s*=>\\s*\\{[^\\}]*if\\s*\\(typeof [^\\.]*.machine\\.preset\\s*!==\\s*"string"\\)' 

# Test 2: Search for tasks that have the `config.machine` preset applied. Expect: Occurrences of tasks with the `config.machine` preset.
rg --type typescript -g '!node_modules' -g '!dist' $'return\\s*\\{\\s*\\.\\.\\.task,\\s*machine:\\s*\\{\\s*preset:\\s*config\\.machine\\s*\\}\\s*\\};'

Length of output: 364


Script:

#!/bin/bash
# Description: Verify the impact of applying the `config.machine` preset to tasks.

# Test 1: Search for tasks that don't have a machine preset defined.
rg -g '*.ts' -g '!node_modules' -g '!dist' 'typeof.*machine.*preset.*!==.*"string"'

# Test 2: Search for tasks that have the `config.machine` preset applied.
rg -g '*.ts' -g '!node_modules' -g '!dist' 'return.*\.\.\.task.*machine:.*preset:.*config\.machine'

# Test 3: Search for other occurrences of `config.machine` for context
rg -g '*.ts' -g '!node_modules' -g '!dist' 'config\.machine'

Length of output: 543

references/v3-catalog/src/trigger/simple.ts (1)

14-14: LGTM!

The addition of the machine property with a preset value of "small-1x" aligns with the PR objective of ensuring that the default machine preset is correctly utilized for all applicable tasks. This change enhances the task's configurability and allows it to be tailored for different operational environments or performance requirements.

It addresses the issue where the default machine preset specified in the configuration file was not being applied to tasks that did not have a machine preset already defined, thereby improving the functionality and reliability of the configuration settings within the project.

packages/cli-v3/src/entryPoints/dev-run-worker.ts (1)

222-223: LGTM!

The changes enhance the error object by adding message and stackTrace properties, providing more detailed information about the error. This aligns with best practices for error handling and reporting, making it easier to diagnose and fix issues related to importing task modules.

packages/cli-v3/src/entryPoints/deploy-run-worker.ts (2)

222-222: Improved error message for missing tasks.

The added error message provides valuable context to the user by specifying the task ID that could not be found. It also guides the user to verify that the task is correctly exported and the ID is accurate. This additional information can help users quickly identify and resolve issues related to missing or incorrectly referenced tasks, enhancing the debugging experience.


252-253: Enhanced error reporting for task import failures.

The changes made to the error handling logic for task import failures are beneficial. By including the original error message and stack trace (when available) in the returned error object, developers gain more context about the failure. This additional information can greatly aid in debugging and understanding the root cause of the import issue.

Furthermore, converting non-Error instances to a string ensures that an error message is always included, even for unexpected error types. This enhances the robustness of the error reporting mechanism.

Overall, these improvements facilitate better troubleshooting and make it easier for developers to diagnose and resolve task import failures.


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai generate interesting stats about this repository and render them as a table.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

pkg-pr-new bot commented Sep 18, 2024

pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev/@trigger.dev/build@1321
pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev@1321
pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev/@trigger.dev/core@1321
pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev/@trigger.dev/sdk@1321

commit: 7b2fba1

@ericallam ericallam merged commit 3b15224 into main Sep 18, 2024
9 checks passed
@ericallam ericallam deleted the v3/missing-import-error-details branch September 18, 2024 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant