Conversation
📝 WalkthroughWalkthroughAdds Vulture dead-code detection (CI workflow, pre-commit hook, pyproject config and whitelist) and removes numerous backend public APIs, DLQ-related flows and tests, native consumer-group monitoring, ResourceCleaner orchestration and providers, various repo/service methods, domain models/schemas, and related tests. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (2)
backend/app/services/result_processor/resource_cleaner.py (2)
23-107:⚠️ Potential issue | 🟠 MajorAll remaining private methods are unreachable dead code.
The ResourceCleaner class is instantiated via the dependency injection container, but with all public methods removed (
cleanup_pod_resources,cleanup_orphaned_resources,get_resource_usage), there are no entry points to invoke any of the remaining private methods. The search confirms:
_cleanup_orphaned_podsand_cleanup_orphaned_configmapshave zero callers anywhere in the codebase_delete_configmapsand_delete_pvcshave zero callers_delete_podis only called internally by_cleanup_orphaned_pods, which itself is unreachable- The class instance is created but never used
The entire class should be removed along with its provider and DI registration, or these private methods need to be re-exposed via new public methods if they're intended to be used elsewhere.
18-21:⚠️ Potential issue | 🟡 Minor
self.networking_v1is unused and should be removed.This attribute is initialized on line 20 but never referenced in any method within the class. Removing it will also eliminate the unnecessary
NetworkingV1Apiinstantiation.🧹 Proposed fix
def __init__(self, api_client: k8s_client.ApiClient, logger: structlog.stdlib.BoundLogger) -> None: self.v1 = k8s_client.CoreV1Api(api_client) - self.networking_v1 = k8s_client.NetworkingV1Api(api_client) self.logger = logger
🧹 Nitpick comments (1)
backend/tests/e2e/events/test_producer_roundtrip.py (1)
12-19: Test has no assertions — consider adding at least a basic one.After the DLQ path removal, this test only calls
producewith no explicit assertion. It relies solely on the absence of an exception, which is a very weak guarantee for an e2e test. Consider asserting on the return value ofproduce(e.g., delivery metadata) or renaming/removing this test if it no longer serves a meaningful purpose.Also, the filename
test_producer_roundtrip.pyno longer reflects the content — there's no "roundtrip" being tested anymore.
…es the ConfigMap with an ownerReference pointing to the pod's UID. K8s garbage collector auto-deletes the ConfigMap when the pod is deleted (by anyone — saga compensation, manual kubectl delete, namespace cleanup, etc.) 2. Simplified handle_delete_pod_command — only deletes the pod now; ConfigMap cleanup is handled by K8s GC 3. Removed networking_v1 — unused client (PR review comment) 4. _create_pod returns the created pod — needed to get the pod UID for the ownerReference
… zero assertions after the DLQ path was removed. No roundtrip, no verification, no value.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/app/services/k8s_worker/worker.py`:
- Around line 128-135: _created_pod can be None when _create_pod hit a 409 (pod
already exists), leaving a newly-created ConfigMap orphaned; update the control
flow after calling _create_pod so that if created_pod is falsy you read the
existing pod (e.g. add a call like self._read_pod/_get_pod using
pod.metadata.name and pod.metadata.namespace), check the returned
pod.metadata.uid, and then call _set_configmap_owner(config_map, existing_pod)
just like in the created case; keep the subsequent await
self._publish_pod_created(command, pod) behavior unchanged and ensure you guard
against missing metadata/uid before calling _set_configmap_owner.
|
1 participant
Summary by cubic
Added Vulture dead code checks to CI and pre-commit and removed unused backend code and tests to reduce maintenance and stabilize builds. Simplified Kubernetes pod cleanup by setting ConfigMap ownerReferences to pods so K8s garbage-collects them automatically; updated README and docs (DLQ now persists to MongoDB, added dead-code badge, CI docs include Vulture).
New Features
Refactors
Written for commit 153392e. Summary will update on new commits.
Summary by CodeRabbit