fix: Latest active release check takes into account release channels#349
fix: Latest active release check takes into account release channels#349adityachoudhari26 merged 3 commits intomainfrom
Conversation
WalkthroughThis change streamlines the release sequencing logic. A new asynchronous function, Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant P as Release Policy Checker
participant F as isReleaseLatestActiveForEnvironment
participant DB as Database
C->>P: Validate new release
P->>F: Check if release is the latest active for environment
F->>DB: Query environment & policy data
DB-->>F: Return environment and policy details
F->>DB: Fetch latest active release with deployment & environment criteria
DB-->>F: Return latest active release record
F-->>P: Return true/false based on release recency
P-->>C: Return policy check result
Possibly related PRs
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
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/job-dispatch/src/policies/release-sequencing.ts (2)
83-151: Handle potential creation date ties and confirm environment lookup logic.If multiple releases share the same
createdAt, this method relies solely oncreatedAtfor comparison. Meanwhile, the earlier policyisPassingNoActiveJobsPolicyuses a tiebreak on version if multiple releases are created at the same time. Consider aligning these tie-breaking approaches for consistency.Also, verify that returning
falsewhen the environment is not found is indeed the intended behavior. If the environment is missing or invalid, you might want to log or throw an error rather than silently returningfalse.
170-186: Avoid repeated environment queries for each trigger group.This logic calls
isReleaseLatestActiveForEnvironment(and thus re-queries the environment) once per grouped set of triggers. If the same environment is reused across many triggers, consider caching or memoizing the environment fetch in memory to reduce DB calls.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/job-dispatch/src/policies/release-sequencing.ts(4 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.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
- GitHub Check: Typecheck
🔇 Additional comments (2)
packages/job-dispatch/src/policies/release-sequencing.ts (2)
7-7: No concerns with introducing 'desc' into the import list.
14-14: Usage oftakeFirstOrNulllooks fine.
Summary by CodeRabbit