-
Notifications
You must be signed in to change notification settings - Fork 11
chore: in memory deployment variable value resource selector #670
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
Conversation
WalkthroughAdds an in-memory selector for DeploymentVariableValue-to-Resource matching, wires it into SelectorManager, integrates selector updates into pipeline upsert/remove flows, and initializes it in workspace creation. Synchronizes matches to a computed DB table and updates control flow to keep selector state consistent with repository changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Pipeline
participant SelectorManager
participant DVVSelector as InMemoryDeploymentVariableValueResourceSelector
participant DB
rect rgb(240,248,255)
note over Pipeline,SelectorManager: Upsert DeploymentVariableValue
Client->>Pipeline: upsertDeploymentVariableValue()
Pipeline->>SelectorManager: upsertDeploymentVariableValue(dvv)
SelectorManager->>DVVSelector: upsertSelector(dvv)
DVVSelector->>DVVSelector: recompute matches (entities ↔ dvv)
DVVSelector->>DB: delete stale + insert new computedDeploymentResource
SelectorManager-->>Pipeline: ok
Pipeline->>Pipeline: mark release targets stale
end
sequenceDiagram
autonumber
participant Client
participant Pipeline
participant SelectorManager
participant DVVSelector as InMemoryDeploymentVariableValueResourceSelector
participant DB
rect rgb(255,248,240)
note over Pipeline,SelectorManager: Remove DeploymentVariableValue
Client->>Pipeline: removeDeploymentVariableValue()
Pipeline->>SelectorManager: removeDeploymentVariableValue(dvv)
SelectorManager->>DVVSelector: removeSelector(dvv)
DVVSelector->>DVVSelector: drop matches in-memory
DVVSelector->>DB: delete computedDeploymentResource rows
SelectorManager-->>Pipeline: ok
Pipeline->>Pipeline: mark release targets stale
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/event-queue/src/selector/in-memory/deployment-variable-value-resource.ts(1 hunks)apps/event-queue/src/selector/selector.ts(5 hunks)apps/event-queue/src/workspace/pipeline.ts(2 hunks)apps/event-queue/src/workspace/workspace.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript with explicit types (prefer interfaces for public APIs)
Import styles: Use named imports, group imports by source (std lib > external > internal)
Consistent type imports:import type { Type } from "module"
Prefer async/await over raw promises
Handle errors explicitly (use try/catch and typed error responses)
Files:
apps/event-queue/src/selector/selector.tsapps/event-queue/src/workspace/workspace.tsapps/event-queue/src/workspace/pipeline.tsapps/event-queue/src/selector/in-memory/deployment-variable-value-resource.ts
⚙️ CodeRabbit configuration file
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
Files:
apps/event-queue/src/selector/selector.tsapps/event-queue/src/workspace/workspace.tsapps/event-queue/src/workspace/pipeline.tsapps/event-queue/src/selector/in-memory/deployment-variable-value-resource.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Formatting: Prettier is used with
@ctrlplane/prettier-config
Files:
apps/event-queue/src/selector/selector.tsapps/event-queue/src/workspace/workspace.tsapps/event-queue/src/workspace/pipeline.tsapps/event-queue/src/selector/in-memory/deployment-variable-value-resource.ts
🧬 Code graph analysis (3)
apps/event-queue/src/selector/selector.ts (3)
packages/db/src/schema/deployment-variables.ts (1)
DeploymentVariableValue(174-176)packages/events/src/kafka/events.ts (1)
FullResource(62-64)apps/event-queue/src/workspace/pipeline.ts (2)
resource(54-57)deploymentVariableValue(86-91)
apps/event-queue/src/workspace/workspace.ts (2)
apps/event-queue/src/selector/selector.ts (1)
deploymentVariableValueResourceSelector(60-62)apps/event-queue/src/selector/in-memory/deployment-variable-value-resource.ts (1)
InMemoryDeploymentVariableValueResourceSelector(25-278)
apps/event-queue/src/selector/in-memory/deployment-variable-value-resource.ts (4)
packages/events/src/kafka/events.ts (1)
FullResource(62-64)packages/db/src/schema/deployment-variables.ts (1)
DeploymentVariableValue(174-176)apps/event-queue/src/selector/in-memory/resource-match.ts (1)
resourceMatchesSelector(32-87)apps/event-queue/src/selector/selector.ts (1)
Selector(11-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Typecheck
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
| if (newlyMatchedDeployments.length > 0) | ||
| await dbClient | ||
| .insert(schema.computedDeploymentResource) | ||
| .values( | ||
| newlyMatchedDeployments.map((deploymentId) => ({ | ||
| resourceId: entity.id, | ||
| deploymentId, | ||
| })), | ||
| ) | ||
| .onConflictDoNothing(); | ||
| } |
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.
Persist actual deployment IDs in computedDeploymentResource.
newlyMatchedDeployments holds selector.id, which is the deployment-variable-value ID. Writing that into computedDeploymentResource.deploymentId stores the wrong key (and will break any FK / downstream lookup that expects a real deployment ID). We need to persist the owning deployment’s ID instead—derive it when loading/upserting selectors (and dedupe when multiple values belong to the same deployment) and use that value for both the insert and delete paths here and below.
🤖 Prompt for AI Agents
In apps/event-queue/src/selector/in-memory/deployment-variable-value-resource.ts
around lines 157 to 167, the code is inserting selector.id
(deployment-variable-value ID) into computedDeploymentResource.deploymentId
which is incorrect; instead derive and persist the owning deployment's ID when
selectors are loaded/upserted, map each selector.value entry to its parent
deploymentId, dedupe so a deployment appears only once even if multiple selector
values belong to it, and use that deduped deploymentId list for the insert here
and for any delete/cleanup paths elsewhere so FK lookups use real deployment
IDs.
Summary by CodeRabbit
New Features
Performance
Reliability
Bug Fixes