chore: migrate relationship rules to db#824
Open
adityachoudhari26 wants to merge 16 commits intomainfrom
Open
Conversation
…roper file formatting
… job mapping; modify OpenAPI paths for deployment variable management and streamline deployment-related endpoints
… to clean up code and improve readability
…dels and queries; enhance job management with unique release IDs
… and related queries; update job retrieval and insertion logic to support new field
…hance job retrieval by release ID; implement GetByReleaseID method in job repository
|
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new RelationshipRule entity end-to-end: DB schema and migration, SQL queries and sqlc mappings, generated Go accessors, repository interfaces and DB/in-memory implementations, store wiring and migration, and Drizzle/TypeScript schema exports. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Store
participant RelRepo as RelationshipRuleRepo
participant DB as Database
rect rgba(0, 150, 200, 0.5)
Note over Client,DB: Get Operation
Client->>Store: Get(id)
Store->>RelRepo: Get(id)
RelRepo->>DB: GetRelationshipRuleByID(id)
DB-->>RelRepo: row
RelRepo->>RelRepo: ToOapi(row)
RelRepo-->>Store: *oapi.RelationshipRule
Store-->>Client: *oapi.RelationshipRule
end
rect rgba(150, 100, 200, 0.5)
Note over Client,DB: Upsert Operation
Client->>Store: Upsert(relationship)
Store->>RelRepo: Set(relationship)
RelRepo->>RelRepo: ToUpsertParams(relationship)
RelRepo->>DB: UpsertRelationshipRule(params)
DB-->>RelRepo: success/error
RelRepo-->>Store: error?
Store-->>Client: success/error
end
rect rgba(200, 100, 100, 0.5)
Note over Client,DB: List Operation
Client->>Store: Items()
Store->>RelRepo: Items()
RelRepo->>DB: ListRelationshipRulesByWorkspaceID(wsID)
DB-->>RelRepo: []rows
RelRepo->>RelRepo: map ToOapi(each row)
RelRepo-->>Store: map[string]*oapi.RelationshipRule
Store-->>Client: map[string]*oapi.RelationshipRule
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
apps/workspace-engine/pkg/workspace/store/repository/db/relationshiprules/repo.go (1)
55-74:Items()silently returns empty map on errors.While logging is present, callers cannot distinguish "no rules exist" from "failed to fetch rules." This could lead to incorrect behavior if a caller assumes an empty map means the workspace has no rules when in reality the DB query failed.
Based on learnings, the user prefers simplicity, so if this behavior is intentional for resilience, consider documenting this contract on the interface.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/pkg/workspace/store/repository/db/relationshiprules/repo.go` around lines 55 - 74, Repo.Items currently swallows errors and returns an empty map, making callers unable to distinguish "no rules" vs "DB/parse failure"; change the Items signature to return (map[string]*oapi.RelationshipRule, error), update the implementation to return a non-nil error when uuid.Parse(r.workspaceID) fails or when db.GetQueries(...).ListRelationshipRulesByWorkspaceID(...) returns an error, and keep the successful path returning the populated map and nil error (use ToOapi(row) unchanged). Also update all callers of Repo.Items to handle the error (and tests) or, if you intend to keep the silent resilience, document this contract on the interface and add a comment above Repo.Items to make the behavior explicit.apps/workspace-engine/pkg/workspace/store/relationships.go (2)
40-51: Unusedctxparameter inRemove.Same observation as
Upsert—the context is not propagated to repository calls. Consider aligning the signature with actual usage or updating the repository interface to accept context for future-proofing (e.g., for tracing, cancellation).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/pkg/workspace/store/relationships.go` around lines 40 - 51, The Remove method on RelationshipRules currently accepts a ctx but never uses it; either remove the ctx parameter from Remove to match current repository methods, or update the repository interface and implementations so repo.Get and repo.Remove accept context and then pass ctx through (i.e., change signatures for repo.Get and repo.Remove and call r.repo.Get(ctx, id) / r.repo.Remove(ctx, id) before calling r.store.changeset.RecordDelete(relationship)). Ensure you update all implementations and tests that call RelationshipRules.Remove or the repo methods to match the new signatures.
27-34: Unusedctxparameter inUpsert.The
ctxparameter is passed but not used in the method body—r.repo.Setdoes not accept a context. If the repository interface doesn't require context propagation, consider removing it from the signature for consistency, or pass it through if the interface is expected to evolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/pkg/workspace/store/relationships.go` around lines 27 - 34, The Upsert method on RelationshipRules currently takes an unused ctx parameter; remove the unused context from the signature to keep the API consistent (or if you intend to propagate context, change r.repo.Set to accept a context and forward ctx into it). Concretely, update RelationshipRules.Upsert to remove the ctx parameter (and remove the context import if unused), adjust all callers to call Upsert(relationship) instead of Upsert(ctx, relationship), and ensure behavior around r.repo.Set and r.store.changeset.RecordUpsert remains unchanged; alternatively, if choosing context propagation, change the repo.Set signature to Set(ctx context.Context, relationship *oapi.RelationshipRule) and forward ctx from RelationshipRules.Upsert into r.repo.Set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/workspace-engine/pkg/db/queries/relationship_rules.sql`:
- Around line 14-18: The ON CONFLICT (id) DO UPDATE currently overwrites rows
across workspaces and even updates workspace_id; change the DO UPDATE to stop
setting workspace_id and to only apply the update when the existing row belongs
to the same workspace (add a WHERE clause that checks
relationship_rules.workspace_id = EXCLUDED.workspace_id). In short: remove
workspace_id from the SET list in the ON CONFLICT handler and add a WHERE
condition on the DO UPDATE to guard updates to rows that match the same
workspace.
In
`@apps/workspace-engine/pkg/workspace/store/repository/db/relationshiprules/mapper.go`:
- Around line 13-21: The selectorFromString function silently swallows errors
from json.Marshal and sel.UnmarshalJSON which can return a partially initialized
oapi.Selector; change selectorFromString to surface failures: make its signature
return (*oapi.Selector, error), check and return errors from
json.Marshal(oapi.CelSelector{Cel: s}) and from sel.UnmarshalJSON(celJSON), and
update callers to handle the error (or, if you prefer fallback behavior, log the
marshal/unmarshal errors at debug/warn before returning nil). Reference:
selectorFromString, oapi.Selector, json.Marshal, sel.UnmarshalJSON.
- Around line 45-46: The code silently ignores JSON unmarshal errors for the
Matcher field: when calling json.Unmarshal(row.Matcher, &matcher) the error is
discarded, which can hide corrupted/invalid data; update the mapper (around
matcher / RelationshipRule_Matcher handling) to capture the returned error, and
on failure log a warning or error (including the raw row.Matcher and the error)
via the package logger or processLogger so callers can detect bad data and
debugging info is preserved; ensure the mapper still returns a sensible zero
value or propagates the error depending on existing function error-handling
conventions.
- Around line 23-32: selectorToString currently only checks sel.AsCelSelector()
and returns empty on failure, losing JsonSelector data; update it to also
attempt sel.AsJsonSelector() when AsCelSelector() fails and return a stable
string representation of the JSON selector (e.g., compact JSON by marshaling the
JsonSelector value or using its Raw/Value field if present). Ensure the updated
logic still returns "" for nil sel, still prefers CelSelector when present
(using cel.Cel), and falls back to the JsonSelector string when available (use
json.Marshal on the JsonSelector struct or its inner field and return the
resulting string, handling any marshal error by returning "").
In
`@apps/workspace-engine/pkg/workspace/store/repository/db/relationshiprules/repo.go`:
- Around line 29-32: The Get method in repo.go is swallowing DB errors from
GetRelationshipRuleByID and returning (nil, false), making callers unable to
tell "not found" vs DB failure; update Get (the method that calls
db.GetQueries(r.ctx).GetRelationshipRuleByID) to surface the error: if
GetRelationshipRuleByID returns an error, log it (using the repository logger)
and return (nil, false, err) or propagate the error to the caller consistent
with the repo interface (or at minimum return an explicit error along with the
not-found flag) so DB/network/timeout failures are not silently discarded.
---
Nitpick comments:
In `@apps/workspace-engine/pkg/workspace/store/relationships.go`:
- Around line 40-51: The Remove method on RelationshipRules currently accepts a
ctx but never uses it; either remove the ctx parameter from Remove to match
current repository methods, or update the repository interface and
implementations so repo.Get and repo.Remove accept context and then pass ctx
through (i.e., change signatures for repo.Get and repo.Remove and call
r.repo.Get(ctx, id) / r.repo.Remove(ctx, id) before calling
r.store.changeset.RecordDelete(relationship)). Ensure you update all
implementations and tests that call RelationshipRules.Remove or the repo methods
to match the new signatures.
- Around line 27-34: The Upsert method on RelationshipRules currently takes an
unused ctx parameter; remove the unused context from the signature to keep the
API consistent (or if you intend to propagate context, change r.repo.Set to
accept a context and forward ctx into it). Concretely, update
RelationshipRules.Upsert to remove the ctx parameter (and remove the context
import if unused), adjust all callers to call Upsert(relationship) instead of
Upsert(ctx, relationship), and ensure behavior around r.repo.Set and
r.store.changeset.RecordUpsert remains unchanged; alternatively, if choosing
context propagation, change the repo.Set signature to Set(ctx context.Context,
relationship *oapi.RelationshipRule) and forward ctx from
RelationshipRules.Upsert into r.repo.Set.
In
`@apps/workspace-engine/pkg/workspace/store/repository/db/relationshiprules/repo.go`:
- Around line 55-74: Repo.Items currently swallows errors and returns an empty
map, making callers unable to distinguish "no rules" vs "DB/parse failure";
change the Items signature to return (map[string]*oapi.RelationshipRule, error),
update the implementation to return a non-nil error when
uuid.Parse(r.workspaceID) fails or when
db.GetQueries(...).ListRelationshipRulesByWorkspaceID(...) returns an error, and
keep the successful path returning the populated map and nil error (use
ToOapi(row) unchanged). Also update all callers of Repo.Items to handle the
error (and tests) or, if you intend to keep the silent resilience, document this
contract on the interface and add a comment above Repo.Items to make the
behavior explicit.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (21)
apps/workspace-engine/pkg/db/models.goapps/workspace-engine/pkg/db/queries/relationship_rules.sqlapps/workspace-engine/pkg/db/queries/schema.sqlapps/workspace-engine/pkg/db/relationship_rules.sql.goapps/workspace-engine/pkg/db/sqlc.yamlapps/workspace-engine/pkg/persistence/integration_test.goapps/workspace-engine/pkg/workspace/store/relationships.goapps/workspace-engine/pkg/workspace/store/repository/db/relationshiprules/mapper.goapps/workspace-engine/pkg/workspace/store/repository/db/relationshiprules/repo.goapps/workspace-engine/pkg/workspace/store/repository/db/repo.goapps/workspace-engine/pkg/workspace/store/repository/interfaces.goapps/workspace-engine/pkg/workspace/store/repository/memory/repo.goapps/workspace-engine/pkg/workspace/store/repository/repo.goapps/workspace-engine/pkg/workspace/store/store.goapps/workspace-engine/svc/workspaceconsumer/consumer.goapps/workspace-engine/test/integration/dbtest.gopackages/db/drizzle/0160_futuristic_catseye.sqlpackages/db/drizzle/meta/0160_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/index.tspackages/db/src/schema/relationship-rule.ts
Comment on lines
+14
to
+18
| ON CONFLICT (id) DO UPDATE | ||
| SET name = EXCLUDED.name, description = EXCLUDED.description, workspace_id = EXCLUDED.workspace_id, | ||
| from_type = EXCLUDED.from_type, to_type = EXCLUDED.to_type, relationship_type = EXCLUDED.relationship_type, | ||
| reference = EXCLUDED.reference, from_selector = EXCLUDED.from_selector, to_selector = EXCLUDED.to_selector, | ||
| matcher = EXCLUDED.matcher, metadata = EXCLUDED.metadata; |
Contributor
There was a problem hiding this comment.
Guard upsert conflicts against cross-workspace overwrites.
On conflict, Line 15 currently updates workspace_id and all fields for any matching id. Add a workspace guard and avoid updating workspace_id itself.
Suggested SQL change
ON CONFLICT (id) DO UPDATE
-SET name = EXCLUDED.name, description = EXCLUDED.description, workspace_id = EXCLUDED.workspace_id,
+SET name = EXCLUDED.name, description = EXCLUDED.description,
from_type = EXCLUDED.from_type, to_type = EXCLUDED.to_type, relationship_type = EXCLUDED.relationship_type,
reference = EXCLUDED.reference, from_selector = EXCLUDED.from_selector, to_selector = EXCLUDED.to_selector,
- matcher = EXCLUDED.matcher, metadata = EXCLUDED.metadata;
+ matcher = EXCLUDED.matcher, metadata = EXCLUDED.metadata
+WHERE relationship_rule.workspace_id = EXCLUDED.workspace_id;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ON CONFLICT (id) DO UPDATE | |
| SET name = EXCLUDED.name, description = EXCLUDED.description, workspace_id = EXCLUDED.workspace_id, | |
| from_type = EXCLUDED.from_type, to_type = EXCLUDED.to_type, relationship_type = EXCLUDED.relationship_type, | |
| reference = EXCLUDED.reference, from_selector = EXCLUDED.from_selector, to_selector = EXCLUDED.to_selector, | |
| matcher = EXCLUDED.matcher, metadata = EXCLUDED.metadata; | |
| ON CONFLICT (id) DO UPDATE | |
| SET name = EXCLUDED.name, description = EXCLUDED.description, | |
| from_type = EXCLUDED.from_type, to_type = EXCLUDED.to_type, relationship_type = EXCLUDED.relationship_type, | |
| reference = EXCLUDED.reference, from_selector = EXCLUDED.from_selector, to_selector = EXCLUDED.to_selector, | |
| matcher = EXCLUDED.matcher, metadata = EXCLUDED.metadata | |
| WHERE relationship_rule.workspace_id = EXCLUDED.workspace_id; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/workspace-engine/pkg/db/queries/relationship_rules.sql` around lines 14
- 18, The ON CONFLICT (id) DO UPDATE currently overwrites rows across workspaces
and even updates workspace_id; change the DO UPDATE to stop setting workspace_id
and to only apply the update when the existing row belongs to the same workspace
(add a WHERE clause that checks relationship_rules.workspace_id =
EXCLUDED.workspace_id). In short: remove workspace_id from the SET list in the
ON CONFLICT handler and add a WHERE condition on the DO UPDATE to guard updates
to rows that match the same workspace.
Comment on lines
+13
to
+21
| func selectorFromString(s string) *oapi.Selector { | ||
| if s == "" { | ||
| return nil | ||
| } | ||
| sel := &oapi.Selector{} | ||
| celJSON, _ := json.Marshal(oapi.CelSelector{Cel: s}) | ||
| _ = sel.UnmarshalJSON(celJSON) | ||
| return sel | ||
| } |
Contributor
There was a problem hiding this comment.
Silent error handling may mask issues.
Both json.Marshal and sel.UnmarshalJSON errors are discarded. If the input string s contains an invalid CEL expression or if marshaling fails, this returns a partially initialized Selector without indication of failure.
Consider logging at debug/warn level if this is intentional fallback behavior, or returning an error if callers should handle invalid selectors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/workspace-engine/pkg/workspace/store/repository/db/relationshiprules/mapper.go`
around lines 13 - 21, The selectorFromString function silently swallows errors
from json.Marshal and sel.UnmarshalJSON which can return a partially initialized
oapi.Selector; change selectorFromString to surface failures: make its signature
return (*oapi.Selector, error), check and return errors from
json.Marshal(oapi.CelSelector{Cel: s}) and from sel.UnmarshalJSON(celJSON), and
update callers to handle the error (or, if you prefer fallback behavior, log the
marshal/unmarshal errors at debug/warn before returning nil). Reference:
selectorFromString, oapi.Selector, json.Marshal, sel.UnmarshalJSON.
Comment on lines
+23
to
+32
| func selectorToString(sel *oapi.Selector) string { | ||
| if sel == nil { | ||
| return "" | ||
| } | ||
| cel, err := sel.AsCelSelector() | ||
| if err == nil && cel.Cel != "" { | ||
| return cel.Cel | ||
| } | ||
| return "" | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find where oapi.Selector is defined and check the struct
rg -n "type Selector" --type=go -A 10 | head -50Repository: ctrlplanedev/ctrlplane
Length of output: 3881
🏁 Script executed:
#!/bin/bash
# Check for all As* methods on Selector types
rg -n "func.*\*Selector.*As[A-Z]" --type=goRepository: ctrlplanedev/ctrlplane
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Look at the mapper.go file itself to understand context
cat -n apps/workspace-engine/pkg/workspace/store/repository/db/relationshiprules/mapper.go | head -50Repository: ctrlplanedev/ctrlplane
Length of output: 1421
🏁 Script executed:
#!/bin/bash
# Find where selectorToString is actually used
rg -n "selectorToString" --type=go -B 2 -A 2Repository: ctrlplanedev/ctrlplane
Length of output: 5149
🏁 Script executed:
#!/bin/bash
# Get more context around the Selector definition in oapi.gen.go
sed -n '1025,1200p' apps/workspace-engine/pkg/oapi/oapi.gen.goRepository: ctrlplanedev/ctrlplane
Length of output: 6114
🏁 Script executed:
#!/bin/bash
# Search for AsCelSelector and other As methods
rg -n "AsCelSelector|func.*As[A-Z].*Selector" --type=go -A 5 | head -100Repository: ctrlplanedev/ctrlplane
Length of output: 7796
🏁 Script executed:
#!/bin/bash
# Look for all selector-related types in oapi.gen.go
rg -n "type.*Selector" apps/workspace-engine/pkg/oapi/oapi.gen.goRepository: ctrlplanedev/ctrlplane
Length of output: 484
🏁 Script executed:
#!/bin/bash
# Search for JsonSelector instantiation or usage in relationship rules context
rg -n "JsonSelector" --type=go | head -30Repository: ctrlplanedev/ctrlplane
Length of output: 4327
🏁 Script executed:
#!/bin/bash
# Check if there's any handling of JsonSelector anywhere in the codebase
rg -n "AsJsonSelector" --type=goRepository: ctrlplanedev/ctrlplane
Length of output: 683
selectorToString does not handle JsonSelector variant.
The Selector union has two variants: CelSelector and JsonSelector. This function only attempts AsCelSelector() and silently returns an empty string if it fails, which causes data loss when a JsonSelector is passed. Tests and production code (e.g., pkg/selector/match.go, versionselector.go) demonstrate that JsonSelector is actively used.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/workspace-engine/pkg/workspace/store/repository/db/relationshiprules/mapper.go`
around lines 23 - 32, selectorToString currently only checks sel.AsCelSelector()
and returns empty on failure, losing JsonSelector data; update it to also
attempt sel.AsJsonSelector() when AsCelSelector() fails and return a stable
string representation of the JSON selector (e.g., compact JSON by marshaling the
JsonSelector value or using its Raw/Value field if present). Ensure the updated
logic still returns "" for nil sel, still prefers CelSelector when present
(using cel.Cel), and falls back to the JsonSelector string when available (use
json.Marshal on the JsonSelector struct or its inner field and return the
resulting string, handling any marshal error by returning "").
Comment on lines
+45
to
+46
| matcher := oapi.RelationshipRule_Matcher{} | ||
| _ = json.Unmarshal(row.Matcher, &matcher) |
Contributor
There was a problem hiding this comment.
Silent unmarshal error for Matcher field.
If row.Matcher contains invalid JSON, the error is discarded and an empty RelationshipRule_Matcher is returned. This could mask data corruption issues.
Consider at minimum logging a warning when unmarshal fails:
🛡️ Proposed fix
matcher := oapi.RelationshipRule_Matcher{}
- _ = json.Unmarshal(row.Matcher, &matcher)
+ if err := json.Unmarshal(row.Matcher, &matcher); err != nil {
+ log.Warn("Failed to unmarshal matcher", "id", row.ID, "error", err)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| matcher := oapi.RelationshipRule_Matcher{} | |
| _ = json.Unmarshal(row.Matcher, &matcher) | |
| matcher := oapi.RelationshipRule_Matcher{} | |
| if err := json.Unmarshal(row.Matcher, &matcher); err != nil { | |
| log.Warn("Failed to unmarshal matcher", "id", row.ID, "error", err) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/workspace-engine/pkg/workspace/store/repository/db/relationshiprules/mapper.go`
around lines 45 - 46, The code silently ignores JSON unmarshal errors for the
Matcher field: when calling json.Unmarshal(row.Matcher, &matcher) the error is
discarded, which can hide corrupted/invalid data; update the mapper (around
matcher / RelationshipRule_Matcher handling) to capture the returned error, and
on failure log a warning or error (including the raw row.Matcher and the error)
via the package logger or processLogger so callers can detect bad data and
debugging info is preserved; ensure the mapper still returns a sensible zero
value or propagates the error depending on existing function error-handling
conventions.
Comment on lines
+29
to
+32
| row, err := db.GetQueries(r.ctx).GetRelationshipRuleByID(r.ctx, uid) | ||
| if err != nil { | ||
| return nil, false | ||
| } |
Contributor
There was a problem hiding this comment.
DB errors silently discarded in Get.
When GetRelationshipRuleByID fails (network issue, connection timeout, etc.), the error is swallowed and callers cannot distinguish "not found" from "DB failure." Consider returning an error or at minimum logging:
🛡️ Proposed fix to log DB errors
row, err := db.GetQueries(r.ctx).GetRelationshipRuleByID(r.ctx, uid)
if err != nil {
+ if err != pgx.ErrNoRows {
+ log.Warn("Failed to get relationship rule", "id", id, "error", err)
+ }
return nil, false
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/workspace-engine/pkg/workspace/store/repository/db/relationshiprules/repo.go`
around lines 29 - 32, The Get method in repo.go is swallowing DB errors from
GetRelationshipRuleByID and returning (nil, false), making callers unable to
tell "not found" vs DB failure; update Get (the method that calls
db.GetQueries(r.ctx).GetRelationshipRuleByID) to surface the error: if
GetRelationshipRuleByID returns an error, log it (using the repository logger)
and return (nil, false, err) or propagate the error to the caller consistent
with the repo interface (or at minimum return an explicit error along with the
not-found flag) so DB/network/timeout failures are not silently discarded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit