Conversation
📝 WalkthroughWalkthroughThe changes rename references from "result service" to "storage service" across nginx configuration generation and cleanup utilities, including variable names, proxy targets, comments, and pod lookup logic. No public APIs or function signatures are modified. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related issues
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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/k8s/kubernetes.py (1)
387-441:⚠️ Potential issue | 🟠 MajorGuard against non-unique or missing storage-service lookup.
get_k8s_resource_namesmay returnNoneorlist[str], which would render an invalidproxy_passtarget. Consider enforcing a single service name before generating the config.🔧 Proposed fix
storage_service_name = get_k8s_resource_names('service', 'label', 'component=flame-storage-service', namespace=namespace) + if not isinstance(storage_service_name, str): + raise ValueError( + f"Expected a single storage-service name, got: {storage_service_name!r}" + )
🤖 Fix all issues with AI agents
In `@src/resources/utils.py`:
- Around line 215-222: get_k8s_resource_names may return None or a list; before
calling delete_resource you must normalize storage_service_name to a list, skip
when empty/None, and call delete_resource for each pod name. Specifically, in
the block that handles cleanup_type (where storage_service_name is assigned from
get_k8s_resource_names), replace the direct delete_resource call with logic
that: (1) checks if storage_service_name is truthy, (2) if it's a string convert
it into a single-item list, (3) iterate the list and call delete_resource(name,
'pod', namespace) for each entry, and (4) set response_content[cleanup_type]
only after performing deletions (or set a helpful message if no pods were
found). Ensure you reference the existing symbols get_k8s_resource_names,
delete_resource, storage_service_name, cleanup_type, namespace, and
response_content.
| if cleanup_type in ['all', 'services', 'rs']: | ||
| # reinitialize result-service pod | ||
| result_service_name = get_k8s_resource_names('pod', | ||
| # reinitialize storage-service pod | ||
| storage_service_name = get_k8s_resource_names('pod', | ||
| 'label', | ||
| 'component=flame-result-service', | ||
| 'component=flame-storage-service', | ||
| namespace=namespace) | ||
| delete_resource(result_service_name, 'pod', namespace) | ||
| response_content[cleanup_type] = "Reset result service" | ||
| delete_resource(storage_service_name, 'pod', namespace) | ||
| response_content[cleanup_type] = "Reset storage service" |
There was a problem hiding this comment.
Handle multiple or missing storage-service pods during cleanup.
get_k8s_resource_names('pod', ...) can return None or list[str]. Passing either to delete_resource will fail. Consider normalizing to a list and skipping when empty.
🔧 Proposed fix
- storage_service_name = get_k8s_resource_names('pod',
- 'label',
- 'component=flame-storage-service',
- namespace=namespace)
- delete_resource(storage_service_name, 'pod', namespace)
- response_content[cleanup_type] = "Reset storage service"
+ storage_service_names = get_k8s_resource_names('pod',
+ 'label',
+ 'component=flame-storage-service',
+ namespace=namespace)
+ if storage_service_names is None:
+ response_content[cleanup_type] = "No storage service pod found"
+ else:
+ if isinstance(storage_service_names, str):
+ storage_service_names = [storage_service_names]
+ for pod_name in storage_service_names:
+ delete_resource(pod_name, 'pod', namespace)
+ response_content[cleanup_type] = "Reset storage service"🤖 Prompt for AI Agents
In `@src/resources/utils.py` around lines 215 - 222, get_k8s_resource_names may
return None or a list; before calling delete_resource you must normalize
storage_service_name to a list, skip when empty/None, and call delete_resource
for each pod name. Specifically, in the block that handles cleanup_type (where
storage_service_name is assigned from get_k8s_resource_names), replace the
direct delete_resource call with logic that: (1) checks if storage_service_name
is truthy, (2) if it's a string convert it into a single-item list, (3) iterate
the list and call delete_resource(name, 'pod', namespace) for each entry, and
(4) set response_content[cleanup_type] only after performing deletions (or set a
helpful message if no pods were found). Ensure you reference the existing
symbols get_k8s_resource_names, delete_resource, storage_service_name,
cleanup_type, namespace, and response_content.
|
Changes have been applied in #41 (no clue why this branch was left loose) |
Summary by CodeRabbit