New Resource worker #450
New Resource worker #450
Conversation
WalkthroughThis pull request updates multiple files to extend resource handling capabilities. The changes include an update to the Changes
Sequence Diagram(s)sequenceDiagram
participant J as Job Queue
participant W as newResourceWorker
participant DB as Database
participant RM as Release Manager
participant Q as Evaluation Queue
J->>W: Submit new resource job
W->>DB: Query environments & deployments
DB-->>W: Return query results
W->>DB: Check for existing release targets (with onConflictDoNothing)
W->>RM: Upsert and mark release as desired
RM-->>W: Confirm release update
W->>Q: Enqueue evaluation job data in bulk
W->>W: Log operation details
Poem
✨ Finishing Touches
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. 🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/event-worker/src/workers/new-resource.ts (1)
70-75: Confirm error handling when callingReleaseManager.Currently, if
releaseManager.upsertVersionReleasefails due to an invalid version or database error, the worker may fail silently or re-attempt indefinitely. Ensure that job failure or retry logic is clear for these scenarios.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.gitignore(1 hunks)apps/event-worker/src/workers/index.ts(2 hunks)apps/event-worker/src/workers/new-resource.ts(1 hunks)packages/events/src/types.ts(2 hunks)packages/release-manager/src/resource/types.ts(1 hunks)packages/validators/src/events/index.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{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...
**/*.{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.
apps/event-worker/src/workers/index.tspackages/validators/src/events/index.tspackages/release-manager/src/resource/types.tspackages/events/src/types.tsapps/event-worker/src/workers/new-resource.ts
🧬 Code Definitions (1)
apps/event-worker/src/workers/index.ts (1)
apps/event-worker/src/workers/new-resource.ts (1)
newResourceWorker(8-90)
🔇 Additional comments (10)
.gitignore (1)
62-63: LGTM: Good addition to ignore VSCode SQL session files.The addition of this rule to ignore
.session.sqlfiles is a good practice for keeping SQL tool connection files out of version control.apps/event-worker/src/workers/index.ts (2)
5-5: LGTM: Good import for the new resource worker.The import statement correctly references the new worker module with the proper file extension.
20-20: LGTM: Proper integration of new resource worker.The new resource worker is correctly mapped to the corresponding Channel.NewResource enum value, maintaining consistency with the existing pattern for worker registration.
packages/events/src/types.ts (2)
16-16: LGTM: New resource channel properly defined.The new channel enum value follows the existing naming convention and is appropriately placed in the enum list.
30-30: LGTM: Proper type mapping for new resource channel.The type mapping for the new resource channel correctly uses the resource schema's inferred select type, which is consistent with the pattern used for other entity channels.
packages/validators/src/events/index.ts (2)
13-13: LGTM: New environment resource event channel added.The new enum value for environment resource events follows the existing naming convention and fits well with the related event types.
57-61: LGTM: Event schema properly defined with required fields.The new event schema correctly defines the structure with the required environment and resource IDs. The type export follows the established pattern in this file.
apps/event-worker/src/workers/new-resource.ts (2)
60-62: Validate the schema for unique constraints before usingonConflictDoNothing().Using
onConflictDoNothing()is effective to avoid duplicate inserts if there is a suitable unique constraint or index on the table. Verify that such constraints exist onschema.releaseTargetto ensure idempotence.
66-68: Handle the case wheredeployment_versionis not found.When looking up
deployment_version.idviadeployments.find(...), the code defaults to"unknown"if no match is found. Ensure subsequent operations can handle an"unknown"version gracefully or consider throwing an error, based on your desired logic.packages/release-manager/src/resource/types.ts (1)
4-42: Interface definitions look solid.The new
ResourceRepositoryinterface is well-documented and clarifies important release operations. Consider adding usage examples or references to these methods in the doc comments if future contributors might need extra guidance.
| for (const { environment } of environments) { | ||
| for (const { deployment } of deployments) { | ||
| const res = await db.select().from(schema.resource) | ||
| .where( | ||
| and( | ||
| eq(schema.resource.id, resource.id), | ||
| schema.resourceMatchesMetadata( | ||
| db, | ||
| environment.resourceSelector, | ||
| ), | ||
| schema.resourceMatchesMetadata( | ||
| db, | ||
| deployment.resourceSelector, | ||
| ), | ||
| ), | ||
| ).then(takeFirstOrNull); | ||
| if (res != null) { | ||
| releaseRepos.push({ | ||
| resourceId: res.id, | ||
| environmentId: environment.id, | ||
| deploymentId: deployment.id, | ||
| }); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider optimizing the nested loops for performance.
Running a database query within nested loops can result in performance bottlenecks, especially if the number of environments and deployments grows. Consider refactoring this into a single combined query or a batched approach to reduce the number of round trips to the database.
Summary by CodeRabbit
New Features
Chores