feat: interactive tree view for resource wait display#583
Open
bsquizz wants to merge 1 commit into
Open
Conversation
Replace the static spinner with a full-screen Textual TUI that shows a live tree of namespace resources while waiting for readiness. The tree displays parent resources (ClowdEnvironments, ClowdApps) with their child components (Deployments, StatefulSets), each with a status indicator and Kubernetes condition text. A progress bar at the bottom shows overall readiness percentage with a countdown timer. Key changes: - Add ResourceWaitApp using textual for scrollable, event-driven tree - Run wait logic in a daemon thread so the TUI is a detachable view layer - Support exiting tree view (ctrl+q) and re-entering it (ctrl+t) - Filter tree to only show relevant ClowdEnvironments - Show resource status conditions next to pending resources - Add "Starting resource tree view..." spinner during watcher init Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Summary by CodeRabbit
WalkthroughThis pull request integrates an interactive resource tree view into namespace/resource readiness flows. It adds watcher reuse infrastructure, implements a Rich/Textual-based interactive display with hierarchy visualization and progress tracking, and updates three CLI commands (namespace wait-on-resources, deploy, deploy-env) to use the new watcher and display functions instead of simple spinners. ChangesResource Watcher and Interactive Display Integration
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bonfire/output.py (1)
413-708:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply Ruff formatting changes before merge.
CI is currently failing because
ruff-formatreformatsbonfire/output.py. This is merge-blocking and should be fixed in this PR.🤖 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 `@bonfire/output.py` around lines 413 - 708, This file fails CI because ruff-format changed formatting; run ruff format (or your pre-commit formatter) against bonfire/output.py and commit the resulting formatting changes so the file matches ruff's style; focus on reformatting the affected functions and classes such as _resource_status_summary, _resource_label_text, _build_hierarchy, _make_tree_app_class, and resource_wait_display (or run ruff --fix on the repo and commit) to resolve the merge-blocking formatting differences.Source: Pipeline failures
🤖 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 `@bonfire/output.py`:
- Around line 585-590: The recursive helper _add_nodes dereferences resource.uid
unguarded; modify it to first retrieve uid via getattr(resource, "uid", None)
(or check hasattr) and only iterate children_map.get(uid, []) when uid is not
None/truthy to avoid crashes for resources missing uid; keep the rest of the
logic (creating node, setting self._node_map[resource.key], calling _add_nodes
recursively) unchanged and reference children_map and _build_hierarchy as the
surrounding context.
- Around line 547-557: The footer readiness counts in _refresh_status currently
compute total/ready from self._watcher.resources filtered only by
_SKIPPED_TREE_RESTYPES, which can differ from the tree/progress because
_build_hierarchy applies additional filtering (e.g., ClowdEnvironment
selection); update _refresh_status to reuse the same filtering pipeline as the
tree by calling the same helper used to produce the displayed items (invoke
_build_hierarchy or the shared filter logic) to derive the set of resources to
count, then compute total and ready from that resulting collection and pass them
to _update_status_label so the footer matches the tree/progress.
---
Outside diff comments:
In `@bonfire/output.py`:
- Around line 413-708: This file fails CI because ruff-format changed
formatting; run ruff format (or your pre-commit formatter) against
bonfire/output.py and commit the resulting formatting changes so the file
matches ruff's style; focus on reformatting the affected functions and classes
such as _resource_status_summary, _resource_label_text, _build_hierarchy,
_make_tree_app_class, and resource_wait_display (or run ruff --fix on the repo
and commit) to resolve the merge-blocking formatting differences.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: e860d595-8b8e-47f1-ab82-76d7054f28ac
📒 Files selected for processing (4)
bonfire/bonfire.pybonfire/openshift.pybonfire/output.pypyproject.toml
Comment on lines
+547
to
+557
| def _refresh_status(self) -> None: | ||
| if self._done_event.is_set(): | ||
| self.exit() | ||
| return | ||
| resources_snapshot = self._watcher.resources.copy() | ||
| filtered = { | ||
| k: r for k, r in resources_snapshot.items() if r.restype not in _SKIPPED_TREE_RESTYPES | ||
| } | ||
| total = len(filtered) | ||
| ready = sum(1 for r in filtered.values() if r.ready) | ||
| self._update_status_label(total, ready) |
Contributor
There was a problem hiding this comment.
Keep footer readiness counts consistent with tree filtering.
Line 552-556 recomputes totals with only restype filtering, while tree/progress uses _build_hierarchy filtering (including relevant ClowdEnvironment filtering). This can show inconsistent ready/total values.
Proposed fix
def _refresh_status(self) -> None:
if self._done_event.is_set():
self.exit()
return
resources_snapshot = self._watcher.resources.copy()
- filtered = {
- k: r for k, r in resources_snapshot.items() if r.restype not in _SKIPPED_TREE_RESTYPES
- }
- total = len(filtered)
- ready = sum(1 for r in filtered.values() if r.ready)
+ _, _, total, ready = _build_hierarchy(resources_snapshot)
self._update_status_label(total, ready)🤖 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 `@bonfire/output.py` around lines 547 - 557, The footer readiness counts in
_refresh_status currently compute total/ready from self._watcher.resources
filtered only by _SKIPPED_TREE_RESTYPES, which can differ from the tree/progress
because _build_hierarchy applies additional filtering (e.g., ClowdEnvironment
selection); update _refresh_status to reuse the same filtering pipeline as the
tree by calling the same helper used to produce the displayed items (invoke
_build_hierarchy or the shared filter logic) to derive the set of resources to
count, then compute total and ready from that resulting collection and pass them
to _update_status_label so the footer matches the tree/progress.
Comment on lines
+585
to
+590
| def _add_nodes(parent_node, resource): | ||
| label = _resource_label_text(resource) | ||
| node = parent_node.add(label, data=resource.key, expand=True) | ||
| self._node_map[resource.key] = node | ||
| for child in children_map.get(resource.uid, []): | ||
| _add_nodes(node, child) |
Contributor
There was a problem hiding this comment.
Guard tree recursion against resources without uid.
_build_hierarchy already handles missing uid defensively, but Line 589 dereferences resource.uid unguarded. Any resource lacking uid will crash tree refresh.
Proposed fix
def _add_nodes(parent_node, resource):
label = _resource_label_text(resource)
node = parent_node.add(label, data=resource.key, expand=True)
self._node_map[resource.key] = node
- for child in children_map.get(resource.uid, []):
+ resource_uid = getattr(resource, "uid", None)
+ for child in children_map.get(resource_uid, []):
_add_nodes(node, child)🤖 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 `@bonfire/output.py` around lines 585 - 590, The recursive helper _add_nodes
dereferences resource.uid unguarded; modify it to first retrieve uid via
getattr(resource, "uid", None) (or check hasattr) and only iterate
children_map.get(uid, []) when uid is not None/truthy to avoid crashes for
resources missing uid; keep the rest of the logic (creating node, setting
self._node_map[resource.key], calling _add_nodes recursively) unchanged and
reference children_map and _build_hierarchy as the surrounding context.
| return roots, children, total, ready | ||
|
|
||
|
|
||
| def _make_tree_app_class(): |
Contributor
There was a problem hiding this comment.
for textual its rather uncommon to make app classes in a helper method (that more of a confused web framework things)
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
2 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
Test plan
bonfire deploy <app>— verify tree renders with correct hierarchy, scrolls with arrow/pageup/pagedown, progress bar updates, and resources show ✓ when readybonfire namespace wait-on-resources <ns>— verify tree view works for standalone wait commandbonfire deploy --db-only <app>— verify it still uses the simple spinner (no tree view)BONFIRE_PLAIN_OUTPUT=1 bonfire deploy <app>— verify non-interactive fallback (no TUI)🤖 Generated with Claude Code