fix: cr deletion#1656
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a dedicated deletion handler and moves namespace/secret cleanup into it. Reconcile always installs a cleanup finalizer and returns; on CR deletion, handleDeletion removes secret finalizers, deletes operator-managed secrets/resources or the managed namespace, then removes the controller finalizer. ChangesDeletion Flow Refactoring
Sequence Diagram(s)sequenceDiagram
participant Reconcile
participant handleDeletion
participant K8sSecrets as "Kubernetes Secrets API"
participant K8sWorkloads as "Kubernetes Workloads (Deploy/Service/SA/PVC/NP/Docling/Valkey)"
Reconcile->>Reconcile: detect non-deleting CR
Reconcile->>Reconcile: add openr.ag/namespace-cleanup finalizer, update CR, return
Reconcile->>handleDeletion: subsequent reconcile sees deletionTimestamp
handleDeletion->>K8sSecrets: fetch env and default secrets (remove finalizers, delete when cross-namespace)
K8sSecrets-->>handleDeletion: secret operations complete (NotFound tolerated)
handleDeletion->>K8sWorkloads: if namespace not owned -> deleteResources (delete deployments/services/SA/PVC/NP/Docling/Valkey)
K8sWorkloads-->>handleDeletion: resource deletions logged (errors tolerated)
handleDeletion->>Reconcile: remove CR finalizer and update CR
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
kubernetes/operator/internal/controller/deletion_handler.go (1)
85-125: 💤 Low valueRedundant secret cleanup exists in
deleteResources.Both
handleDeletion(here) anddeleteResources(lines 153-220 in openrag_controller.go) contain logic to clean up .env secrets and default secrets. WhendeleteResourcesis called at line 146, these secrets have already been cleaned up by the code in lines 33-125 above, making the secret cleanup code indeleteResourcesexecute as no-ops (secrets will be NotFound).Consider either:
- Removing the secret cleanup code from
deleteResourcessincehandleDeletionalways runs first, or- Removing it from
handleDeletionand relying ondeleteResources(but would need to make it strict)This duplication doesn't cause bugs but adds maintenance burden and makes the deletion flow harder to reason about.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kubernetes/operator/internal/controller/deletion_handler.go` around lines 85 - 125, The deletion logic for cleaning up generated .env/default secrets is duplicated between handleDeletion (this file: defaultSecretNames loop, controllerutil.ContainsFinalizer checks, r.Delete calls, userSecretFinalizer usage and getOrGenerateSecret naming) and deleteResources in openrag_controller.go; remove the duplicate by deleting the secret-cleanup block from deleteResources (the code that looks up the same defaultSecretNames/env secrets, removes finalizers and deletes them) and keep the current implementation in handleDeletion as the single source of truth so secrets are removed only once; ensure references to getOrGenerateSecret naming convention and userSecretFinalizer remain consistent and update any comments in deleteResources that mention secret cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@kubernetes/operator/internal/controller/deletion_handler.go`:
- Around line 85-125: The deletion logic for cleaning up generated .env/default
secrets is duplicated between handleDeletion (this file: defaultSecretNames
loop, controllerutil.ContainsFinalizer checks, r.Delete calls,
userSecretFinalizer usage and getOrGenerateSecret naming) and deleteResources in
openrag_controller.go; remove the duplicate by deleting the secret-cleanup block
from deleteResources (the code that looks up the same defaultSecretNames/env
secrets, removes finalizers and deletes them) and keep the current
implementation in handleDeletion as the single source of truth so secrets are
removed only once; ensure references to getOrGenerateSecret naming convention
and userSecretFinalizer remain consistent and update any comments in
deleteResources that mention secret cleanup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1d4f2e2c-e1be-4d1d-bb4d-252be3421059
📒 Files selected for processing (2)
kubernetes/operator/internal/controller/deletion_handler.gokubernetes/operator/internal/controller/openrag_controller.go
github-actions
Bot
added
bug
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
kubernetes/operator/internal/controller/deletion_handler.go (1)
149-152: 💤 Low valueSecret cleanup in
deleteResourcesis redundant.When
deleteResourcesis called here, secrets have already been cleaned up by lines 36-128 above. The secret cleanup logic indeleteResources(lines 166-233) will mostly encounterNotFounderrors.Consider either removing the secret cleanup from
deleteResourcessincehandleDeletionalways runs first, or consolidating all secret cleanup intodeleteResourcesand calling it unconditionally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kubernetes/operator/internal/controller/deletion_handler.go` around lines 149 - 152, The secret cleanup is being done twice: once in handleDeletion and again inside deleteResources; pick one approach and update the code accordingly — either remove the secret-cleanup block from deleteResources (so deleteResources only deletes non-secret resources and handleDeletion continues to clean secrets) or move the secret-cleanup logic entirely into deleteResources and ensure callers (including handleDeletion) call deleteResources unconditionally; locate the secret cleanup code inside the deleteResources function and either delete that block or remove the early-return in handleDeletion so it always invokes r.deleteResources(ctx, o, targetNS), and update any comments/tests to reflect the chosen consolidation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@kubernetes/operator/internal/controller/deletion_handler.go`:
- Around line 358-369: The Docling PVC deletion loop currently unconditionally
deletes PVCs for resourceName("ds-data") and resourceName("dw-data"); update it
to respect the same PVCReclaimPolicy used for Langflow PVCs by checking the CR's
PVCReclaimPolicy (or equivalent field) before calling r.Delete — only call
r.Delete(ctx, pvc) when the policy equals the "Delete" behavior, otherwise skip
deletion (and keep the existing logger.Info/Logger.Error paths for attempted
deletes); use the same policy check logic that is used in the Langflow PVC
handling to locate the correct field and semantics.
- Around line 415-428: The PVC cleanup loop in deletion_handler.go currently
hardcodes "for i := 0; i < 1; i++" and builds names with resourceName("valkey"),
which misses PVCs for replica ordinals >0 and uses the wrong PVC base name;
update the deletion logic to (1) determine the actual replica count from the
Valkey StatefulSet (use valkeyStatefulSet.Spec.Replicas or
o.Spec.DoclingComponents.Valkey.Replicas) and iterate i from 0 to replicas-1,
(2) build PVC names using the StatefulSet volumeClaimTemplate name
("valkey-data") combined with the statefulset name and ordinal (the pattern is
<volumeClaimTemplate.Name>-<statefulset-name>-<ordinal>), and (3) only perform
per-ordinal PVC deletion when Valkey is configured to create PVCs (i.e., storage
is enabled and no ExistingClaim is set in the Valkey storage config) to avoid
deleting user-provided claims.
---
Nitpick comments:
In `@kubernetes/operator/internal/controller/deletion_handler.go`:
- Around line 149-152: The secret cleanup is being done twice: once in
handleDeletion and again inside deleteResources; pick one approach and update
the code accordingly — either remove the secret-cleanup block from
deleteResources (so deleteResources only deletes non-secret resources and
handleDeletion continues to clean secrets) or move the secret-cleanup logic
entirely into deleteResources and ensure callers (including handleDeletion) call
deleteResources unconditionally; locate the secret cleanup code inside the
deleteResources function and either delete that block or remove the early-return
in handleDeletion so it always invokes r.deleteResources(ctx, o, targetNS), and
update any comments/tests to reflect the chosen consolidation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 11366656-d5c5-4a7c-b9c0-e3149843803f
📒 Files selected for processing (2)
kubernetes/operator/internal/controller/deletion_handler.gokubernetes/operator/internal/controller/openrag_controller.go
💤 Files with no reviewable changes (1)
- kubernetes/operator/internal/controller/openrag_controller.go
Comment on lines
+358
to
+369
| // Delete Docling PVCs if storage is enabled | ||
| // Note: Could add reclaim policy for Docling PVCs in future | ||
| for _, name := range []string{resourceName("ds-data"), resourceName("dw-data")} { | ||
| pvc := &corev1.PersistentVolumeClaim{} | ||
| err := r.Get(ctx, client.ObjectKey{Name: name, Namespace: targetNS}, pvc) | ||
| if err == nil { | ||
| logger.Info("Deleting Docling PVC", "name", name) | ||
| if err := r.Delete(ctx, pvc); err != nil && !errors.IsNotFound(err) { | ||
| logger.Error(err, "failed to delete Docling PVC", "name", name) | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Docling PVCs deleted unconditionally, unlike Langflow PVC.
Langflow PVC respects PVCReclaimPolicy (lines 271-289), but Docling PVCs are always deleted when DoclingComponents is enabled. This inconsistency could surprise users who expect their Docling data to be preserved on CR deletion.
Consider applying the same PVCReclaimPolicy check, or if Docling components are ephemeral by design, document this behavior difference in the API spec.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@kubernetes/operator/internal/controller/deletion_handler.go` around lines 358
- 369, The Docling PVC deletion loop currently unconditionally deletes PVCs for
resourceName("ds-data") and resourceName("dw-data"); update it to respect the
same PVCReclaimPolicy used for Langflow PVCs by checking the CR's
PVCReclaimPolicy (or equivalent field) before calling r.Delete — only call
r.Delete(ctx, pvc) when the policy equals the "Delete" behavior, otherwise skip
deletion (and keep the existing logger.Info/Logger.Error paths for attempted
deletes); use the same policy check logic that is used in the Langflow PVC
handling to locate the correct field and semantics.
Comment on lines
+415
to
+428
| // Delete Valkey PVCs (from StatefulSet) | ||
| // Note: StatefulSet PVCs have format: data-<statefulset-name>-<ordinal> | ||
| // We should delete these to avoid orphaned PVCs | ||
| for i := 0; i < 1; i++ { // Default replicas is 1 | ||
| pvcName := fmt.Sprintf("data-%s-%d", resourceName("valkey"), i) | ||
| pvc := &corev1.PersistentVolumeClaim{} | ||
| err := r.Get(ctx, client.ObjectKey{Name: pvcName, Namespace: targetNS}, pvc) | ||
| if err == nil { | ||
| logger.Info("Deleting Valkey PVC", "name", pvcName) | ||
| if err := r.Delete(ctx, pvc); err != nil && !errors.IsNotFound(err) { | ||
| logger.Error(err, "failed to delete Valkey PVC", "name", pvcName) | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Valkey replicas is configurable in the OpenRAG spec
ast-grep --pattern 'type Valkey struct {
$$$
}'
# Also check for Replicas field in API types
rg -n 'Replicas' --type go -g '*/api/*'Repository: langflow-ai/openrag
Length of output: 45
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect the deletion handler around the reported lines
echo "=== deletion_handler.go (context) ==="
sed -n '360,480p' kubernetes/operator/internal/controller/deletion_handler.go
# 2) Find where Valkey spec is defined (Valkey struct / fields)
echo
echo "=== Find Valkey type/fields ==="
rg -n "type Valkey|DoclingComponents\.Valkey|DoclingComponents.*Valkey|valkey\.Replicas|Valkey.*Replicas|Replicas.*Valkey" kubernetes/operator -S || true
rg -n "type Valkey struct" . -S || true
# 3) Find where the Valkey StatefulSet is created (replica count used)
echo
echo "=== Find Valkey StatefulSet creation ==="
rg -n "StatefulSet.*Valkey|valkey.*StatefulSet|Kind:.*StatefulSet|NewStatefulSet|replicas" kubernetes/operator/internal/controller -S || true
# 4) Check if there's an API type for replicas under OpenRAG/DoclingComponents
echo
echo "=== Search for replicas under DoclingComponents ==="
rg -n "DoclingComponents|Valkey" . --type go -S | head -n 200 || true
rg -n "Replicas" . -S --type go | head -n 200 || trueRepository: langflow-ai/openrag
Length of output: 23489
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show ValkeySpec fields (around the reported location)
echo "=== kubernetes/operator/api/v1alpha1/openrag_types.go: ValkeySpec ==="
sed -n '400,480p' kubernetes/operator/api/v1alpha1/openrag_types.go
# Show replicasOrDefault implementation
echo
echo "=== replicasOrDefault ==="
sed -n '2360,2410p' kubernetes/operator/internal/controller/openrag_controller.go
# Show valkeyStatefulSet implementation (to confirm replica usage + PVC template name)
echo
echo "=== valkeyStatefulSet (replicas + volumeClaimTemplates) ==="
sed -n '1670,1785p' kubernetes/operator/internal/controller/openrag_controller.goRepository: langflow-ai/openrag
Length of output: 8257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Check ComponentSpec definition (where Replicas likely comes from)
echo "=== ComponentSpec (openrag_types.go) ==="
sed -n '1,120p' kubernetes/operator/api/v1alpha1/openrag_types.go
# 2) Check the remainder of valkeyStatefulSet for volumeClaimTemplates + retention policy
echo
echo "=== valkeyStatefulSet (tail: VolumeClaimTemplates / retention policy) ==="
rg -n "VolumeClaimTemplates|PersistentVolumeClaimRetentionPolicy|RetentionPolicy|volumeClaimTemplates" kubernetes/operator/internal/controller/openrag_controller.go -n
sed -n '1785,1905p' kubernetes/operator/internal/controller/openrag_controller.goRepository: langflow-ai/openrag
Length of output: 9084
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for valkey PVC naming patterns ==="
rg -n "data-\\%s-\\%d|data-%s|valkey-data|volumeClaimTemplates" kubernetes/operator/internal/controller/deletion_handler.go kubernetes/operator/internal/controller/openrag_controller.go
echo
echo "=== Find usage of PVCReclaimPolicy / persistentVolumeClaimRetentionPolicy ==="
rg -n "PVCReclaimPolicy|persistentVolumeClaimRetentionPolicy|ReclaimPolicy" kubernetes/operator/internal/controller kubernetes/operator/api/v1alpha1Repository: langflow-ai/openrag
Length of output: 252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for valkey PVC naming patterns (simple strings) ==="
rg -n "Deleting Valkey PVC|data-valkey|valkey-data|data-.*valkey-data|data-.*valkey" kubernetes/operator/internal/controller
echo
echo "=== Find usage of PVCReclaimPolicy / persistentVolumeClaimRetentionPolicy ==="
rg -n "PVCReclaimPolicy|persistentVolumeClaimRetentionPolicy|ReclaimPolicy" kubernetes/operator/internal/controller kubernetes/operator/api/v1alpha1Repository: langflow-ai/openrag
Length of output: 1936
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect resourceName helper
echo "=== resourceName helper ==="
rg -n "func resourceName\\(" -S kubernetes/operator/internal/controller
rg -n "resourceName\\(" kubernetes/operator/internal/controller/openrag_controller.go kubernetes/operator/internal/controller/deletion_handler.go -n
# Print the resourceName function body (few lines around match)
file="$(rg -n "func resourceName\\(" -S kubernetes/operator/internal/controller | head -n1 | cut -d: -f1)"
line="$(rg -n "func resourceName\\(" -S kubernetes/operator/internal/controller | head -n1 | cut -d: -f2)"
start=$((line-10)); end=$((line+30))
echo
echo "=== Showing ${file}:${start}-${end} ==="
sed -n "${start},${end}p" "$file"
# Inspect deletion_handler.go around the Valkey PVC cleanup and any storage checks
echo
echo "=== deletion_handler.go around Valkey deletion ==="
sed -n '240,460p' kubernetes/operator/internal/controller/deletion_handler.go
# Inspect valkeyStatefulSet volumeClaimTemplates naming and claim template naming (already partially seen)
echo
echo "=== openrag_controller.go around valkeyStatefulSet volumeClaimTemplates ==="
sed -n '1680,1860p' kubernetes/operator/internal/controller/openrag_controller.goRepository: langflow-ai/openrag
Length of output: 22797
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find StatefulSet volumeClaimTemplates names across controller ==="
rg -n "volumeClaimTemplates\\s*=\\s*\\[\\]corev1\\.PersistentVolumeClaim|VolumeClaimTemplates" kubernetes/operator/internal/controller/openrag_controller.go
echo
echo "=== Extract any PersistentVolumeClaim ObjectMeta Name values inside volumeClaimTemplates ==="
rg -n "Name:\\s*\"[^\"]+\"" kubernetes/operator/internal/controller/openrag_controller.go | head -n 200
echo
echo "=== Search for PersistentVolumeClaimTemplate name 'data' usage ==="
rg -n "Name:\\s*\"data\"|data-\\%s|fmt\\.Sprintf\\(\"data-" kubernetes/operator/internal/controller/openrag_controller.go kubernetes/operator/internal/controller/deletion_handler.goRepository: langflow-ai/openrag
Length of output: 4479
Fix Valkey PVC cleanup to delete the right PVCs for all configured replicas
valkeyStatefulSetsetsStatefulSet.Spec.Replicasfromo.Spec.DoclingComponents.Valkey.Replicas(defaulting to 1), but deletion handler hardcodesfor i := 0; i < 1; i++, leaving PVCs for ordinals >0 orphaned.- Deletion handler deletes
data-<statefulset-name>-<ordinal>, but the StatefulSetVolumeClaimTemplatesusesName: "valkey-data"—the cleanup needs to use thevalkey-data-based PVC naming (otherwise even ordinal 0 won’t be deleted). - Consider gating the PVC cleanup to the cases where Valkey actually creates per-ordinal PVCs (storage enabled with no
ExistingClaim).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@kubernetes/operator/internal/controller/deletion_handler.go` around lines 415
- 428, The PVC cleanup loop in deletion_handler.go currently hardcodes "for i :=
0; i < 1; i++" and builds names with resourceName("valkey"), which misses PVCs
for replica ordinals >0 and uses the wrong PVC base name; update the deletion
logic to (1) determine the actual replica count from the Valkey StatefulSet (use
valkeyStatefulSet.Spec.Replicas or o.Spec.DoclingComponents.Valkey.Replicas) and
iterate i from 0 to replicas-1, (2) build PVC names using the StatefulSet
volumeClaimTemplate name ("valkey-data") combined with the statefulset name and
ordinal (the pattern is
<volumeClaimTemplate.Name>-<statefulset-name>-<ordinal>), and (3) only perform
per-ordinal PVC deletion when Valkey is configured to create PVCs (i.e., storage
is enabled and no ExistingClaim is set in the Valkey storage config) to avoid
deleting user-provided claims.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@kubernetes/operator/internal/controller/openrag_controller_test.go`:
- Around line 65-75: The first call to r.Reconcile assigns the variable res but
that value is never used because res is immediately overwritten by the second
call; remove the ineffectual assignment by calling
r.Reconcile(context.Background(), ctrl.Request{...}) without assigning its
return to res (or assign only err and check it) for the first invocation in
openrag_controller_test.go, keeping the require.NoError(t, err) check and
preserving the second reconcile that actually creates resources; update any
references to res so only the second reconcile retains the res variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 39381dcd-63a6-495a-9aae-812e9ae8f3f1
📒 Files selected for processing (2)
kubernetes/operator/internal/controller/deletion_handler.gokubernetes/operator/internal/controller/openrag_controller_test.go
💤 Files with no reviewable changes (1)
- kubernetes/operator/internal/controller/deletion_handler.go
github-actions
Bot
added
bug
github-actions
Bot
added
bug
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kubernetes/operator/internal/controller/deletion_handler.go (1)
145-170:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDefer CR finalizer removal until cross-namespace cleanup errors are surfaced
handleDeletionremoves the CR finalizer right afterr.deleteResources(ctx, o, targetNS), butdeleteResourcesonly logs cleanup failures and always ends withreturn nil; this swallows transient Get/Update/Delete/RBAC errors and prevents the controller from retrying cleanup after the CR is deleted (risking orphaned resources in user-provided namespaces).♻️ Minimal fix direction
+import utilerrors "k8s.io/apimachinery/pkg/util/errors" + func (r *OpenRAGReconciler) deleteResources(ctx context.Context, o *openragv1alpha1.OpenRAG, targetNS string) error { logger := log.FromContext(ctx) + var cleanupErrs []error // ... if err := r.Update(ctx, envSecret); err != nil { logger.Error(err, "failed to remove finalizer from env secret", "name", envSecretName) + cleanupErrs = append(cleanupErrs, fmt.Errorf("remove env secret finalizer %s: %w", envSecretName, err)) } // collect delete/update failures the same way for the remaining resources - return nil + return utilerrors.NewAggregate(cleanupErrs) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kubernetes/operator/internal/controller/deletion_handler.go` around lines 145 - 170, The finalizer is removed regardless of cleanup outcome because deleteResources currently swallows errors; update deleteResources to return meaningful errors (propagate Get/Update/Delete/RBAC failures instead of just logging) and change the deletion flow that calls deleteResources (the block using r.deleteResources(ctx, o, targetNS), controllerutil.RemoveFinalizer(o, finalizer), and r.Update(ctx, o)) so that controllerutil.RemoveFinalizer and r.Update are only invoked after deleteResources returns nil; if deleteResources returns an error, return that error from the handler to trigger a retry rather than removing the finalizer.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@kubernetes/operator/internal/controller/deletion_handler.go`:
- Around line 145-170: The finalizer is removed regardless of cleanup outcome
because deleteResources currently swallows errors; update deleteResources to
return meaningful errors (propagate Get/Update/Delete/RBAC failures instead of
just logging) and change the deletion flow that calls deleteResources (the block
using r.deleteResources(ctx, o, targetNS), controllerutil.RemoveFinalizer(o,
finalizer), and r.Update(ctx, o)) so that controllerutil.RemoveFinalizer and
r.Update are only invoked after deleteResources returns nil; if deleteResources
returns an error, return that error from the handler to trigger a retry rather
than removing the finalizer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06fbe29c-361c-474a-bf69-cb76a1e7a586
📒 Files selected for processing (1)
kubernetes/operator/internal/controller/deletion_handler.go
ricofurtado
pushed a commit
that referenced
this pull request
May 22, 2026
* fix cr deletion * refactoring * fix unit test * fix lint * fix cross namespace CR deletion
ricofurtado
added a commit
that referenced
this pull request
May 22, 2026
…on (#1625) * feat: Implement in-process JWT claims caching with TTL and LRU eviction * style: ruff format (auto) * refactor: Update cache initialization to use settings for max size and TTL * feat: code cleanup * fix: Improve public key file reading and enhance token scheme handling * style: ruff autofix (auto) * fix: Handle user insert races and add test timeout (#1618) * Handle user insert races and add test timeout Increase TypeScript integration test timeout to 120s to reduce flakiness during slow CI runs. Enhance user_service.ensure_user_row IntegrityError handling to explicitly handle concurrent-insert races: detect a (oauth_provider, oauth_subject) race and return the existing row, detect an email_lookup_hash race by looking up the email and returning the concurrent identity when it matches, and handle PK collisions by retrying the insert with a new UUID. Add explanatory comments about which collisions are recoverable and when errors should propagate to the caller. * style: ruff format (auto) * Use PEP 604 union for agent config return type Replace typing.Optional[dict] with PEP 604 union syntax (dict | None) for get_effective_agent_config and remove the now-unused Optional import. This is a pure type-annotation cleanup (no runtime behavior changes); note it requires Python 3.10+ for the `|` union syntax. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * fix: display file names and fix ingestion for onedrive (#1609) * fixed onedrive not working * style: ruff format (auto) * removed sites.read.all from onedrive * style: ruff format (auto) * fixed allowed users and groups * fix lint error * fixed lint * fixed mypy lint * style: ruff format (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * fix: Skip deleted connector files; return orphan IDs (#1592) * Skip deleted connector files; return orphan IDs Handle files deleted at source and improve orphan reconciliation. - Add TaskStatus.SKIPPED and update ConnectorFileProcessor to catch FileNotFoundError/ValueError from connector.get_file_content. When a file is missing, mark the file task as SKIPPED, set a skipped result, update timestamps and counters, and avoid raising so the upload can continue. - Change reconcile_orphans_for_connector_type to return a list of orphan file IDs ([]) instead of an int, update early returns and docstring to reflect returning the deleted/removed document IDs so callers can exclude them from subsequent sync passes. - Minor whitespace cleanup in sync_all_connectors. These changes avoid aborting syncs on missing source files and make orphan deletions explicit for downstream processing. * Skip deleted connector files; return orphan IDs Handle files deleted at source and improve orphan reconciliation. - Add TaskStatus.SKIPPED and update ConnectorFileProcessor to catch FileNotFoundError/ValueError from connector.get_file_content. When a file is missing, mark the file task as SKIPPED, set a skipped result, update timestamps and counters, and avoid raising so the upload can continue. - Change reconcile_orphans_for_connector_type to return a list of orphan file IDs ([]) instead of an int, update early returns and docstring to reflect returning the deleted/removed document IDs so callers can exclude them from subsequent sync passes. - Minor whitespace cleanup in sync_all_connectors. These changes avoid aborting syncs on missing source files and make orphan deletions explicit for downstream processing. * Add *.db to .gitignore Add a '*.db' pattern to .gitignore to prevent local database files (e.g., SQLite) from being committed to the repository. * Add orphan reconcile and bulk-delete helper Introduce a reconcile pass and supporting bulk-delete utility to remove OpenSearch chunks for files that no longer exist remotely. - Add reconcile_orphans_for_connector_type (src/api/connectors.py): lists all active connections for a connector type, strictly gates on unauthenticated connections or listing errors (abort with 0 deletes), aggregates paginated remote file IDs, computes orphans (indexed IDs not present remotely) and invokes delete_chunks_by_document_ids. Integrated into connector_sync and sync_all_connectors (skips reconcile when sync is capped). - Add delete_chunks_by_document_ids (src/api/documents.py): issues a single delete_by_query with terms(document_id, ...) and conflicts="proceed", returns deleted count, and short-circuits on empty input. - Update connector metadata painless params (src/connectors/service.py): include filename param and assign ctx._source.filename = params.filename in the update script so renamed files update indexed chunks. - Add unit tests: cover delete_chunks_by_document_ids, reconcile_orphans_for_connector_type (gating, pagination, multi-connection union, error handling), and connector metadata filename behavior. Behavior notes: reconcile is conservative to avoid false-positive deletions; bulk delete is defensive and returns 0 on unexpected responses or failures. * fix lint * style: ruff format (auto) * fix lint * Use max_files param and add typing Add an explicit type annotation for files_to_process and update the connector.list_files call to use the max_files parameter instead of limit to match the connector API. Also remove redundant `# type: ignore` comments on connector.cfg assignments as they are no longer necessary. Minor cleanup to improve type clarity and API consistency. * Add connector sync preview UI and API Introduce sync-preview functionality for connectors and sync-all flows. Backend: add ID→filename aggregation, compute_orphans (non-destructive orphan detection), orphan deletion helper, and preview endpoints (connector_sync_preview, connectors_sync_all_preview) plus wiring in internal routes. Frontend: add useSyncConnectorPreview/useSyncAllConnectorsPreview mutations, wire preview flows into Knowledge pages/components, and add SyncConfirmDialog component to show orphan lists, per-connector synced counts, availability flags, and confirm deletion+sync interactions. Also update UI buttons to open the preview dialog and handle loading/syncing states. * style: ruff format (auto) * Hoist and consolidate imports in processors Move many inline imports to module scope and consolidate repeated imports across TaskProcessor and its subclasses. Adds top-level imports (asyncio, datetime, mimetypes, os, time), config/settings and utility functions (clients, get_embedding_model, get_index_name, get_openrag_config, extract_relevant, process_text_file, resplit_chunks_character_windows, ensure_embedding_field_exists, auto_cleanup_tempfile, hash_id, build_filename_delete_body, build_filename_search_body) and imports TaskStatus. Removes redundant per-function imports to improve readability and reduce repeated import overhead; no functional changes intended. * Type UseQueryOptions with SearchResult Specify UseQueryOptions<SearchResult> for the useGetSearchQuery hook's options parameter to improve TypeScript type inference and ensure the query options expect SearchResult data. This change tightens typings without altering runtime behavior. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * fix: add reset db on command, expose error on e2e tests (#1627) * fix factory reset not deleting data * make onboarding errors pop up on e2e tests * style: ruff format (auto) * added check for error when uploading * added check if its on first step to rollback, not only if its complete * reset correctly * update timeout for uploading document * remove misclick * fixed lint * fix mypy errors * style: ruff format (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * refactor: handler for file upload for context (#1624) * file upload for context * coderabbit suggestions * chore: add operator make commands (#1628) * Add kind build/load targets and kind sample Add Makefile targets to build and load app images into a kind cluster (KIND_CLUSTER_NAME default: openrag): kind-load-app-images and kind-build-load-apps, and expose them in the help output. Update kubernetes/operator/README.md with instructions to run the operator locally, apply a kind-local sample, build/load images into kind, and restart deployments after rebuilding. Add a new config sample openrag_v1alpha1_openrag-kind-local.yaml tuned for 2-CPU kind/Colima clusters (lower resource requests, imagePullPolicy: Never). Also annotate the default sample to point users to the kind-local file for local clusters. * Add Makefile help for operator & kind Add a new help_operator target and OPERATOR_DIR variable to the Makefile, and include it in the .PHONY list. The new help output documents Kubernetes operator and kind-related commands (build/load app images into kind, operator deps/install/run/build/test/lint/manifests/generate, deploy/undeploy, docker-build, helm install) and provides a typical kind + local images workflow and sample CR usage. This makes it easier for developers to discover and run local operator/cluster tasks from the repo root. * redirect on logout (#1440) (#1636) * fix: Copy flows directory into Docker image (#1632) Add a COPY instruction to include the local flows/ directory at /app/flows in the image so bundled flows are available at runtime. Placed before the entrypoint/ownership handling to ensure the files are copied with the intended UID/permissions. * feat: settings saas tabs menu (#1637) * remove logic for previous tab selected, need to be discussed with Ana * reduce repeated code and use darkmodelight class * reduce repeated code and use darkmodelight class * remove unecessary !important * remove unecessary !important * remove duplicate css blocks * remove !important and merge active and hover state * put back TabsContent anf fix hover issue --------- Co-authored-by: Olfa Maslah <[email protected]> * chore: Add ruff autofix step to CI workflows (#1640) * Add ruff autofix step to CI workflows Add an in-place Ruff fix step to .github/workflows/autofix.ci.yml that runs uv run ruff check --fix on changed files, rename the job and autofix commit message to reflect "ruff autofix", and update comments to clarify that autofix.ci applies safe fixes and formatting. Also update .github/workflows/lint-backend.yml comments to state that safe lint fixes and formatting are auto-applied by autofix.ci while keeping strict lint (--no-fix) and mypy in the lint workflow. * Update autofix.ci.yml * fix: Enum IDs and delete OpenSearch docs by _id (#1638) * Enum IDs and delete OpenSearch docs by _id Add DLS-safe OpenSearch delete helpers and use them to avoid silent no-ops from delete_by_query. Introduces utils/opensearch_delete.py with collect_visible_document_ids and delete_document_ids (enumerate visible _ids via search/scroll, then delete by primary _id). Update delete_chunks_by_document_ids to enumerate chunk IDs and delete each by _id. Ensure langflow_connector_service and TaskProcessor clear stale chunks (by document_id) before re-ingest/re-index to prevent duplicate or trailing chunks after renames. Improve connector listing by scoping cfg.file_ids/folder_ids when available to avoid false orphan detection. Add and update unit tests to assert the new enumeration-and-delete behavior and ordering. * style: ruff format (auto) * ruff fix * ruff fix --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * make probes more robust (#1642) * feat: Show google_drive connector for cloud brand (#1650) Remove google_drive from the cloud-brand exclusion so Google Drive connectors are visible when isCloudBrand is true. Applied the change in connector-cards.tsx and knowledge-dropdown.tsx (OneDrive remains excluded). * operator ubi base build (#1652) * remove flow download initContainer when flowRef is removed (#1653) * fix: cr deletion (#1656) * fix cr deletion * refactoring * fix unit test * fix lint * fix cross namespace CR deletion * Potential fix for pull request finding 'CodeQL / Information exposure through an exception' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Edwin Jose <[email protected]> Co-authored-by: Lucas Oliveira <[email protected]> Co-authored-by: Mike Fortman <[email protected]> Co-authored-by: Wallgau <[email protected]> Co-authored-by: Olfa Maslah <[email protected]> Co-authored-by: ming <[email protected]> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
ricofurtado
pushed a commit
that referenced
this pull request
May 23, 2026
* fix cr deletion * refactoring * fix unit test * fix lint * fix cross namespace CR deletion
ricofurtado
added a commit
that referenced
this pull request
May 23, 2026
…on (#1625) * feat: Implement in-process JWT claims caching with TTL and LRU eviction * style: ruff format (auto) * refactor: Update cache initialization to use settings for max size and TTL * feat: code cleanup * fix: Improve public key file reading and enhance token scheme handling * style: ruff autofix (auto) * fix: Handle user insert races and add test timeout (#1618) * Handle user insert races and add test timeout Increase TypeScript integration test timeout to 120s to reduce flakiness during slow CI runs. Enhance user_service.ensure_user_row IntegrityError handling to explicitly handle concurrent-insert races: detect a (oauth_provider, oauth_subject) race and return the existing row, detect an email_lookup_hash race by looking up the email and returning the concurrent identity when it matches, and handle PK collisions by retrying the insert with a new UUID. Add explanatory comments about which collisions are recoverable and when errors should propagate to the caller. * style: ruff format (auto) * Use PEP 604 union for agent config return type Replace typing.Optional[dict] with PEP 604 union syntax (dict | None) for get_effective_agent_config and remove the now-unused Optional import. This is a pure type-annotation cleanup (no runtime behavior changes); note it requires Python 3.10+ for the `|` union syntax. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * fix: display file names and fix ingestion for onedrive (#1609) * fixed onedrive not working * style: ruff format (auto) * removed sites.read.all from onedrive * style: ruff format (auto) * fixed allowed users and groups * fix lint error * fixed lint * fixed mypy lint * style: ruff format (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * fix: Skip deleted connector files; return orphan IDs (#1592) * Skip deleted connector files; return orphan IDs Handle files deleted at source and improve orphan reconciliation. - Add TaskStatus.SKIPPED and update ConnectorFileProcessor to catch FileNotFoundError/ValueError from connector.get_file_content. When a file is missing, mark the file task as SKIPPED, set a skipped result, update timestamps and counters, and avoid raising so the upload can continue. - Change reconcile_orphans_for_connector_type to return a list of orphan file IDs ([]) instead of an int, update early returns and docstring to reflect returning the deleted/removed document IDs so callers can exclude them from subsequent sync passes. - Minor whitespace cleanup in sync_all_connectors. These changes avoid aborting syncs on missing source files and make orphan deletions explicit for downstream processing. * Skip deleted connector files; return orphan IDs Handle files deleted at source and improve orphan reconciliation. - Add TaskStatus.SKIPPED and update ConnectorFileProcessor to catch FileNotFoundError/ValueError from connector.get_file_content. When a file is missing, mark the file task as SKIPPED, set a skipped result, update timestamps and counters, and avoid raising so the upload can continue. - Change reconcile_orphans_for_connector_type to return a list of orphan file IDs ([]) instead of an int, update early returns and docstring to reflect returning the deleted/removed document IDs so callers can exclude them from subsequent sync passes. - Minor whitespace cleanup in sync_all_connectors. These changes avoid aborting syncs on missing source files and make orphan deletions explicit for downstream processing. * Add *.db to .gitignore Add a '*.db' pattern to .gitignore to prevent local database files (e.g., SQLite) from being committed to the repository. * Add orphan reconcile and bulk-delete helper Introduce a reconcile pass and supporting bulk-delete utility to remove OpenSearch chunks for files that no longer exist remotely. - Add reconcile_orphans_for_connector_type (src/api/connectors.py): lists all active connections for a connector type, strictly gates on unauthenticated connections or listing errors (abort with 0 deletes), aggregates paginated remote file IDs, computes orphans (indexed IDs not present remotely) and invokes delete_chunks_by_document_ids. Integrated into connector_sync and sync_all_connectors (skips reconcile when sync is capped). - Add delete_chunks_by_document_ids (src/api/documents.py): issues a single delete_by_query with terms(document_id, ...) and conflicts="proceed", returns deleted count, and short-circuits on empty input. - Update connector metadata painless params (src/connectors/service.py): include filename param and assign ctx._source.filename = params.filename in the update script so renamed files update indexed chunks. - Add unit tests: cover delete_chunks_by_document_ids, reconcile_orphans_for_connector_type (gating, pagination, multi-connection union, error handling), and connector metadata filename behavior. Behavior notes: reconcile is conservative to avoid false-positive deletions; bulk delete is defensive and returns 0 on unexpected responses or failures. * fix lint * style: ruff format (auto) * fix lint * Use max_files param and add typing Add an explicit type annotation for files_to_process and update the connector.list_files call to use the max_files parameter instead of limit to match the connector API. Also remove redundant `# type: ignore` comments on connector.cfg assignments as they are no longer necessary. Minor cleanup to improve type clarity and API consistency. * Add connector sync preview UI and API Introduce sync-preview functionality for connectors and sync-all flows. Backend: add ID→filename aggregation, compute_orphans (non-destructive orphan detection), orphan deletion helper, and preview endpoints (connector_sync_preview, connectors_sync_all_preview) plus wiring in internal routes. Frontend: add useSyncConnectorPreview/useSyncAllConnectorsPreview mutations, wire preview flows into Knowledge pages/components, and add SyncConfirmDialog component to show orphan lists, per-connector synced counts, availability flags, and confirm deletion+sync interactions. Also update UI buttons to open the preview dialog and handle loading/syncing states. * style: ruff format (auto) * Hoist and consolidate imports in processors Move many inline imports to module scope and consolidate repeated imports across TaskProcessor and its subclasses. Adds top-level imports (asyncio, datetime, mimetypes, os, time), config/settings and utility functions (clients, get_embedding_model, get_index_name, get_openrag_config, extract_relevant, process_text_file, resplit_chunks_character_windows, ensure_embedding_field_exists, auto_cleanup_tempfile, hash_id, build_filename_delete_body, build_filename_search_body) and imports TaskStatus. Removes redundant per-function imports to improve readability and reduce repeated import overhead; no functional changes intended. * Type UseQueryOptions with SearchResult Specify UseQueryOptions<SearchResult> for the useGetSearchQuery hook's options parameter to improve TypeScript type inference and ensure the query options expect SearchResult data. This change tightens typings without altering runtime behavior. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * fix: add reset db on command, expose error on e2e tests (#1627) * fix factory reset not deleting data * make onboarding errors pop up on e2e tests * style: ruff format (auto) * added check for error when uploading * added check if its on first step to rollback, not only if its complete * reset correctly * update timeout for uploading document * remove misclick * fixed lint * fix mypy errors * style: ruff format (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * refactor: handler for file upload for context (#1624) * file upload for context * coderabbit suggestions * chore: add operator make commands (#1628) * Add kind build/load targets and kind sample Add Makefile targets to build and load app images into a kind cluster (KIND_CLUSTER_NAME default: openrag): kind-load-app-images and kind-build-load-apps, and expose them in the help output. Update kubernetes/operator/README.md with instructions to run the operator locally, apply a kind-local sample, build/load images into kind, and restart deployments after rebuilding. Add a new config sample openrag_v1alpha1_openrag-kind-local.yaml tuned for 2-CPU kind/Colima clusters (lower resource requests, imagePullPolicy: Never). Also annotate the default sample to point users to the kind-local file for local clusters. * Add Makefile help for operator & kind Add a new help_operator target and OPERATOR_DIR variable to the Makefile, and include it in the .PHONY list. The new help output documents Kubernetes operator and kind-related commands (build/load app images into kind, operator deps/install/run/build/test/lint/manifests/generate, deploy/undeploy, docker-build, helm install) and provides a typical kind + local images workflow and sample CR usage. This makes it easier for developers to discover and run local operator/cluster tasks from the repo root. * redirect on logout (#1440) (#1636) * fix: Copy flows directory into Docker image (#1632) Add a COPY instruction to include the local flows/ directory at /app/flows in the image so bundled flows are available at runtime. Placed before the entrypoint/ownership handling to ensure the files are copied with the intended UID/permissions. * feat: settings saas tabs menu (#1637) * remove logic for previous tab selected, need to be discussed with Ana * reduce repeated code and use darkmodelight class * reduce repeated code and use darkmodelight class * remove unecessary !important * remove unecessary !important * remove duplicate css blocks * remove !important and merge active and hover state * put back TabsContent anf fix hover issue --------- Co-authored-by: Olfa Maslah <[email protected]> * chore: Add ruff autofix step to CI workflows (#1640) * Add ruff autofix step to CI workflows Add an in-place Ruff fix step to .github/workflows/autofix.ci.yml that runs uv run ruff check --fix on changed files, rename the job and autofix commit message to reflect "ruff autofix", and update comments to clarify that autofix.ci applies safe fixes and formatting. Also update .github/workflows/lint-backend.yml comments to state that safe lint fixes and formatting are auto-applied by autofix.ci while keeping strict lint (--no-fix) and mypy in the lint workflow. * Update autofix.ci.yml * fix: Enum IDs and delete OpenSearch docs by _id (#1638) * Enum IDs and delete OpenSearch docs by _id Add DLS-safe OpenSearch delete helpers and use them to avoid silent no-ops from delete_by_query. Introduces utils/opensearch_delete.py with collect_visible_document_ids and delete_document_ids (enumerate visible _ids via search/scroll, then delete by primary _id). Update delete_chunks_by_document_ids to enumerate chunk IDs and delete each by _id. Ensure langflow_connector_service and TaskProcessor clear stale chunks (by document_id) before re-ingest/re-index to prevent duplicate or trailing chunks after renames. Improve connector listing by scoping cfg.file_ids/folder_ids when available to avoid false orphan detection. Add and update unit tests to assert the new enumeration-and-delete behavior and ordering. * style: ruff format (auto) * ruff fix * ruff fix --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * make probes more robust (#1642) * feat: Show google_drive connector for cloud brand (#1650) Remove google_drive from the cloud-brand exclusion so Google Drive connectors are visible when isCloudBrand is true. Applied the change in connector-cards.tsx and knowledge-dropdown.tsx (OneDrive remains excluded). * operator ubi base build (#1652) * remove flow download initContainer when flowRef is removed (#1653) * fix: cr deletion (#1656) * fix cr deletion * refactoring * fix unit test * fix lint * fix cross namespace CR deletion * Potential fix for pull request finding 'CodeQL / Information exposure through an exception' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Edwin Jose <[email protected]> Co-authored-by: Lucas Oliveira <[email protected]> Co-authored-by: Mike Fortman <[email protected]> Co-authored-by: Wallgau <[email protected]> Co-authored-by: Olfa Maslah <[email protected]> Co-authored-by: ming <[email protected]> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
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
1 participant
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
Bug Fixes
New Features
Tests