-
-
Notifications
You must be signed in to change notification settings - Fork 654
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
Capture and propagate env info (TRIGGER_ENV) for socket and container providers #1733
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 9990b56 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
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 |
WalkthroughThis pull request introduces changes to capture and propagate environment information across multiple components. The updates add an "env" header extraction during socket connections, include the trigger environment as an argument for container creation in both Docker and Kubernetes providers, and update internal schemas and classes to support this new data. The changes are implemented without altering the existing interfaces or removing functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant Worker as ProdWorker
participant Coordinator as TaskCoordinator
Worker->>Coordinator: Initiates socket connection with header "x-trigger-env"
Coordinator-->>Worker: Calls setSocketDataFromHeader("env", "x-trigger-env") to capture env info
Note over Worker,Coordinator: Environment info is now included in the socket data
sequenceDiagram
participant TaskOps as TaskOperations
participant Container as Container Engine
TaskOps->>Container: Create container with env variable TRIGGER_ENV=opts.envType
Container-->>TaskOps: Container initiated with environment variable set
Suggested reviewers
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 (
|
d3a1eb7
to
1a69772
Compare
1a69772
to
244c79e
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: 0
🧹 Nitpick comments (1)
.changeset/hungry-moles-do.md (1)
6-7
: Clarify the Changeset Description.
The description "Add TRIGGER_ENV environment variable to task runs" is clear and succinct. For better traceability and future reference, consider expanding it with more context about how this variable will be used across components or its impact on task execution.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/hungry-moles-do.md
(1 hunks)
🔇 Additional comments (1)
.changeset/hungry-moles-do.md (1)
1-4
: Ensure Release Patch Consistency.
The changeset specifies patch bumps for both"trigger.dev"
and"@trigger.dev/core"
. Please verify that these version updates align with your semantic versioning guidelines and that the context of the changes (i.e., the addition of theTRIGGER_ENV
environment variable) is adequately documented.
This isn't quite the right way to provide a new built-in environment variable to workloads in Trigger.dev. You should instead add the trigger.dev/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts Line 857 in 187200a
You can see in that file in other places where we set similar env vars. Also, there are instructions here for how to contribute and do local testing: https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md |
Fixes #1557
✅ Checklist
Testing
Aside of automatic testing, I tried running this changeset in my local machine and while normal behaviour seems to be kept I didn't find a way to be sure my changes were indeed applied as I don't see the env vars being injected in a dev environment (outside of docker/kubernetes).
Since I didn't seen other related env vars (for example TRIGGER_ENV_ID) I assume it's expected and those vars only get injected in production-like infrastructures.
I am more than happy to perform more extended tests provided any one can point me in the right direction if my asumption is incorrect.
I started the discussion here: https://discord.com/channels/1066956501299777596/1316791509818937354/1344447567286501390
Changelog
Adding a new TRIGGER_ENV env var.
Screenshots
[Screenshots]
💯
Summary by CodeRabbit
TRIGGER_ENV
environment variable to enhance task execution context.