fix: Update concurrency policy stuff #348
Conversation
WalkthroughThis pull request updates both UI and backend components. In the webservice, the deployment component now always renders the CreateReleaseDialog instead of conditionally based on the tab. In the job dispatch package, query logic has been modified: additional inner joins have been introduced in job creation and release sequencing, and conditional checks have been updated in both the job completion and concurrency policy evaluations. No public API changes were made. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CTA as DeploymentCTA
participant CRD as CreateReleaseDialog
U->>CTA: Load Deployment View
CTA->>CRD: Render CreateReleaseDialog (always)
CRD-->>CTA: Dialog rendered
CTA-->>U: Display updated UI
sequenceDiagram
participant JC as JobCompletion Handler
participant DB as Database
participant Env as Environment Table
participant Res as Resource Table
participant CP as Concurrency Policy Evaluator
JC->>DB: Execute updated query (joins with Env & Res)
DB-->>JC: Return enriched job data
JC->>CP: Process job triggers (group by deployment & policy)
CP-->>JC: Return concurrency evaluation result
Suggested Reviewers
Poem
✨ Finishing Touches
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
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: 0
🧹 Nitpick comments (2)
packages/job-dispatch/src/policies/release-sequencing.ts (1)
102-105: Add an index on resource deletion state (if not already present).Joining on
schema.resource.idand filtering byresource.deletedAtcan benefit from an index to handle large data sets efficiently.packages/job-dispatch/src/policies/concurrency-policy.ts (1)
23-57: Group triggers by deployment & policy carefully.The grouping and sorting by
createdAtis clear. However, ifconcurrencyLimitis zero or undefined, ensure it reflects the desired behavior. Confirm the logic for boundary conditions on concurrency settings.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/webservice/src/app/[workspaceSlug]/(appv2)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/_components/DeploymentCTA.tsx(1 hunks)packages/job-dispatch/src/job-creation.ts(2 hunks)packages/job-dispatch/src/policies/concurrency-policy.ts(3 hunks)packages/job-dispatch/src/policies/release-sequencing.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict en...
**/*.{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.
packages/job-dispatch/src/policies/release-sequencing.tsapps/webservice/src/app/[workspaceSlug]/(appv2)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/_components/DeploymentCTA.tsxpackages/job-dispatch/src/policies/concurrency-policy.tspackages/job-dispatch/src/job-creation.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Typecheck
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: Format
🔇 Additional comments (7)
packages/job-dispatch/src/policies/release-sequencing.ts (1)
107-114: Looks good!Ensuring that resources are not soft-deleted aligns with the active status filtering. This change correctly excludes resources marked as deleted.
apps/webservice/src/app/[workspaceSlug]/(appv2)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/_components/DeploymentCTA.tsx (1)
36-43: Always displaying the CreateReleaseDialog.Removing the conditional for the "releases" tab and persistently showing the dialog simplifies the logic. This appears to align with a consistent user experience for creating new releases, regardless of tabs.
packages/job-dispatch/src/job-creation.ts (2)
88-91: Ensure environment references are valid.Joining
schema.environmentis fine, but be certain thatenvironmentIdcan't benull. If so, consider handling this case to avoid unwanted inner-join exclusions.
105-106: Validate non-null policy IDs.Using
schema.environmentPolicy.idrequires thatenvironment.policyIdis reliably set. If an environment can lack a policy, handle such cases to avoid skipping or mismatching triggers.packages/job-dispatch/src/policies/concurrency-policy.ts (3)
3-3: Import additions seem consistent with concurrency checks.No issues: the extra imports (e.g.,
count,isNull,notInArray) complement the concurrency logic below.
59-101: Confirm combined filtering logic for active jobs.This query checks status and
resource.deletedAtfor concurrency constraints. It seems well-structured. Just confirm indexing on these fields to avoid performance issues under high concurrency.
103-118: Graceful slices for concurrency-limited triggers.Slicing triggers up to the calculated allowance is a neat approach. The usage of
Math.max(0, concurrencyLimit - count)avoids negative indexing issues, which is good.
Summary by CodeRabbit
New Features
Refactor