You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "eugenegujing (via GitHub)" <gi...@apache.org> on 2026/08/24 21:08:26 UTC
[PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
eugenegujing opened a new pull request, #7944:
URL: https://github.com/apache/texera/pull/7944
### What changes were proposed in this PR?
When a computing unit becomes unhealthy(the pod is OOM-killed into a crash loop, evicted for disk pressure, stuck on an image pull, or its node becomes unreachable), the UI used to show **"(Connecting)"** with a "starting up" tooltip forever, because `ComputingUnitState` only had `Running` and `Pending` and status resolution only looked at `pod.status.phase`.
This PR follows the decisions settled in #7670: mirror Kubernetes for the status vocabulary; owner-only, actionable failure reasons; no auto-recovery — users delete and recreate (the existing terminate flow already handles dead pods).
#### Screenshots
Two representative states as examples, and the complete set of states and reasons is the mapping table in the next section, and every display combination (status × reason × owner) was verified manually.
An `Unknown` unit (node unreachable): red badge, disabled "Unit Unavailable" run button, and the owner-facing reason on the row tooltip:
<img width="614" height="135" alt="Screenshot 2026-08-23 at 12 21 44 AM" src="https://github.com/user-attachments/assets/dc3f05f4-a723-46d3-8306-29f827fd530b" />
A recovered OOM-killed unit: stays green and runnable, but the tooltip carries the out-of-memory warning with the restart count:
<img width="550" height="241" alt="Screenshot 2026-08-23 at 1 05 13 AM" src="https://github.com/user-attachments/assets/555d440c-411a-4b6f-a970-bef3a7ac2555" />
#### Status vocabulary and reasons
`ComputingUnitState` gains `Failed`, `Unknown`, `Terminating`. The full mapping from observed pod state to status and owner-facing `statusReason`:
| Observed pod state | Status | Owner-facing `statusReason` |
|---|---|---|
| `deletionTimestamp` set | `Terminating` | — |
| phase `Failed` + reason `Evicted`, message mentions ephemeral/disk | `Failed` | "The computing unit was evicted because it ran out of local disk storage. Consider storing less data on the unit's local file system, or recreate it with more storage." |
| phase `Failed` + reason `Evicted`, other | `Failed` | "The computing unit was evicted by the cluster (\<first sentence of the cluster message, capped at 120 chars\>). Consider recreating it." |
| container waiting `ImagePullBackOff` / `ErrImagePull` / `InvalidImageName` | `Failed` | "The computing unit's image could not be pulled. Please recreate the unit or contact an administrator." |
| container waiting `CrashLoopBackOff`, last termination `OOMKilled` | `Failed` | "The computing unit keeps crashing because it runs out of memory. Please terminate it and recreate it with a higher memory limit." |
| container waiting `CrashLoopBackOff`, other | `Failed` | "The computing unit is repeatedly crashing (restarted N times). Please terminate and recreate it, or contact an administrator." |
| phase `Failed`, not evicted | `Failed` | "The computing unit stopped unexpectedly. Please terminate and recreate it, or contact an administrator." |
| phase `Unknown` | `Unknown` | "The state of the computing unit cannot be determined (its node may be unreachable)." |
| phase `Pending` + `PodScheduled=False/Unschedulable` condition | `Pending` | "The computing unit is waiting for cluster resources to become available." |
| phase `Running` + a container's last termination `OOMKilled` | `Running` | "The last run was terminated because the computing unit ran out of memory (restarted N times). Consider recreating the unit with a higher memory limit before running the same workload." |
| phase `Running`, healthy | `Running` | — |
| anything else / pod absent / `local` unit | `Pending` / `Running` (unchanged) | — |
Precedence is top-to-bottom; in particular a `CrashLoopBackOff` failure wins over the recovered-OOM warning, and `Terminating` wins over everything.
`statusReason` is **owner-only**: the backend sends `null` to shared users at every DTO construction site, and the frontend then falls back to a generic text. Frontend fallbacks when `statusReason` is null: `Running` → "Ready to use", `Pending` → "Computing unit is starting up", `Failed`/`Unknown` → "This computing unit is unavailable.", `Terminating` → "Computing unit is shutting down", other states → the status word itself.
#### Backend
- The existing single namespace-level pod listing now yields a `PodStatusSnapshot` per pod (phase, deletionTimestamp, pod reason/message, `Unschedulable` condition, per-container waiting reason / last termination reason / restart count) via a pure, unit-testable transform — **no additional Kubernetes round trips**.
- The restartPolicy subtlety that shaped the design: with `restartPolicy: Always`, an OOM-killed container restarts in place and the pod phase never leaves `Running`, so OOM kills are only visible through `containerStatuses[].lastState.terminated.reason`. A unit that recovered stays `Running` with a warning; a unit that cannot come back up lands in `CrashLoopBackOff` and is reported `Failed`.
- Vanished-pod reconciliation (#6853/#6854) is untouched: it stays keyed purely on pod presence, and a present-but-`Failed` pod is **not** treated as vanished (covered by tests).
- `local` units are unchanged (always `Running`) — local liveness is out of scope here, per the discussion.
#### Frontend
- `status` union widened to the five states; `statusReason?` added.
- `Failed`/`Unknown`: red badge in the dropdown (the previously dead red branch, now reachable).
- No text suffixes next to the unit name for any state: a "(Unavailable)" label truncated at real dropdown widths, and the pre-existing Pending "(Connecting)" suffix was equally redundant (gold badge + tooltip + the run button's own "Connecting" spinner) — status is uniformly conveyed by the badge color and explained by the row tooltip.
- Each dropdown row has exactly **one tooltip surface — the row itself**: hovering the row body shows the status/reason, plus "Cannot select." when the unit is not selectable. The nested badge and name tooltips were removed because they stacked a second bubble on top of the row's (the name's tooltip was the raw pod URI, which stays available in the details modal; the action icons keep their own tooltips, as on main). This also makes the recovered-OOM warning on a `Running` unit reachable by hovering the row rather than a few-pixel dot.
- The run button shows a disabled **"Unit Unavailable"** state instead of the endless "Connecting" spinner when the selected unit is `Failed`/`Unknown` (the spinner branch never consulted unit status).
- The ". Cannot select." concatenation now trims a trailing period off the status tooltip before appending, since a `statusReason` is a full sentence — previously this produced a doubled dot.
### Any related issues, documentation, discussions?
Closes #7669. The design questions (status vocabulary, how much detail reaches which users, auto-recovery) were discussed and settled in #7670.
### How was this PR tested?
- TDD: one backend test per decision-table row using `PodBuilder`-built pods through the pure snapshot + mapping functions, including both eviction wordings, all three image-pull reasons, crash-loop with and without OOM history, the crash-loop-beats-recovered-OOM precedence, multi-container pods, owner gating, and the absent-pod path used by creation polling. fabric8 null-guard paths (`status`, `conditions`, `containerStatuses`, `state`, `lastState`, `terminated`) have dedicated tests.
- Mutation-checked: precedence swap, Evicted→Pending, wording-branch removal, restart-count hardcoding, owner-gate removal, absent-pod→Unknown, and run-button branch removal each make at least one test fail.
- Edge-case sweep on top of the row-by-row tests: Terminating precedence over eviction (and over a status-less pod), eviction-message truncation/whitespace/case-insensitivity, mixed multi-container pods (the crash-looping container's own history decides the wording), stale/malformed `Unschedulable` conditions, image-pull-beats-crash-loop precedence, empty-string statusReason not shadowing the fallback, and a pinning test documenting that phase `Succeeded` maps to `Pending`.
- `ComputingUnitManagingService` util specs 90/90, resource specs 60/60, scalafmt clean; frontend `tsc --noEmit` clean, prettier clean, all changed spec files pass under `ng test`.
- Manually verified every frontend display state (all status × reason × owner combinations) against a live dev stack.
Notes for more information: `Option[String] → null` serialization relies on `DefaultScalaModule`, registered on the service's Dropwizard `ObjectMapper`; there is no end-to-end JSON test for the new field. A bare pod in phase `Succeeded` still maps to `Pending` (pinned by a test) — with `restartPolicy: Always` this phase is practically unreachable, so it is left as a known limitation. Follow-ups deliberately out of scope (per #7670): frontend timeout for the "pod Running but engine unreachable" zombie case, UX for the silent vanish-reconcile, and `local` CU liveness.
### Was this PR authored or co-authored using generative AI tooling?
Co-authored by: Claude Code (Claude Fable 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "eugenegujing (via GitHub)" <gi...@apache.org>.
eugenegujing commented on code in PR #7944:
URL: https://github.com/apache/texera/pull/7944#discussion_r4099854477
##########
frontend/src/app/common/type/computing-unit-connection.interface.ts:
##########
@@ -24,5 +24,8 @@
export enum ComputingUnitState {
Running = "Running",
Pending = "Pending",
+ Failed = "Failed",
+ Unknown = "Unknown",
+ Terminating = "Terminating",
Review Comment:
Fixed in 830f26117.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "eugenegujing (via GitHub)" <gi...@apache.org>.
eugenegujing commented on PR #7944:
URL: https://github.com/apache/texera/pull/7944#issuecomment-5447776233
@kunwp1 PTAL!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "eugenegujing (via GitHub)" <gi...@apache.org>.
eugenegujing commented on code in PR #7944:
URL: https://github.com/apache/texera/pull/7944#discussion_r3877429918
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/util/ComputingUnitHelpers.scala:
##########
@@ -55,42 +62,140 @@ object ComputingUnitHelpers {
}
def getComputingUnitStatus(unit: WorkflowComputingUnit): ComputingUnitState =
- singleUnitStatus(unit, KubernetesClient)
+ getComputingUnitStatusWithReason(unit)._1
+
+ /** Single-unit status plus the owner-facing reason (see [[kubernetesStatusAndReason]]). */
+ def getComputingUnitStatusWithReason(
+ unit: WorkflowComputingUnit
+ ): (ComputingUnitState, Option[String]) =
+ singleUnitStatusAndReason(unit, KubernetesClient)
/**
* Single-unit status via a per-unit pod lookup (a targeted GET, cheaper than listing the whole
* namespace). The client is a by-name parameter — not the global singleton — so the kubernetes
* branch is unit-testable with a stub and the local/unknown branches never force the singleton;
- * the public overload binds the production [[KubernetesClient]]. (Metrics has no analogous seam:
+ * the public overloads bind the production [[KubernetesClient]]. (Metrics has no analogous seam:
* its per-unit lookup already fans out to the whole namespace and the bulk (unit, podMetrics)
* overload already covers the cpu/memory resolution, so nothing there is worth pinning.)
*/
- private[util] def singleUnitStatus(
+ private[util] def singleUnitStatusAndReason(
unit: WorkflowComputingUnit,
k8s: => KubernetesClient
- ): ComputingUnitState = {
+ ): (ComputingUnitState, Option[String]) = {
unit.getType match {
// Local CUs are always “running”
case WorkflowComputingUnitTypeEnum.local =>
- Running
+ (Running, None)
- // Kubernetes CUs – only explicit “Running” counts as running
+ // Kubernetes CUs – resolved from the pod's status snapshot
case WorkflowComputingUnitTypeEnum.kubernetes =>
- // Guard the pod status the same way the bulk getAllPodPhases does: a pod with no
- // status yet has a null getStatus, so map through Option to avoid an NPE.
val client = k8s
- val phaseOpt = client
- .getPodByName(client.generatePodName(unit.getCuid))
- .flatMap(pod => Option(pod.getStatus).map(_.getPhase))
-
- if (phaseOpt.contains("Running")) Running else Pending
+ kubernetesStatusAndReason(
+ client
+ .getPodByName(client.generatePodName(unit.getCuid))
+ .map(PodStatusSnapshot.fromPod)
+ )
// Any other (unknown) type is treated as pending
case _ =>
- Pending
+ (Pending, None)
+ }
+ }
+
+ // Owner-facing wording for each failure mode. Deliberately actionable prose, never a raw
+ // Kubernetes dump; buildDashboardUnit withholds these from non-owners entirely.
+ private val ImagePullWaitingReasons = Set("ImagePullBackOff", "ErrImagePull", "InvalidImageName")
+ private val EvictedDiskReason =
+ "The computing unit was evicted because it ran out of local disk storage. Consider " +
+ "storing less data on the unit's local file system, or recreate it with more storage."
+ private val ImagePullReason =
+ "The computing unit's image could not be pulled. Please recreate the unit or contact " +
+ "an administrator."
+ private val CrashLoopOomReason =
+ "The computing unit keeps crashing because it runs out of memory. Please terminate it " +
+ "and recreate it with a higher memory limit."
+ private val GenericFailedReason =
+ "The computing unit stopped unexpectedly. Please terminate and recreate it, or contact " +
+ "an administrator."
+ private val UnknownStateReason =
+ "The state of the computing unit cannot be determined (its node may be unreachable)."
+ private val UnschedulableReason =
+ "The computing unit is waiting for cluster resources to become available."
+
+ private def evictedReason(podMessage: Option[String]): String = {
+ val mentionsDisk =
+ podMessage.exists { message =>
+ val lower = message.toLowerCase
+ lower.contains("ephemeral") || lower.contains("disk")
+ }
+ if (mentionsDisk) EvictedDiskReason
+ else {
+ // First sentence of the cluster's message, capped so the tooltip stays readable.
+ val shortReason = podMessage
+ .map(_.takeWhile(_ != '.').trim)
+ .filter(_.nonEmpty)
+ .map(sentence => if (sentence.length > 120) sentence.take(120).trim + "..." else sentence)
+ .getOrElse("Evicted")
+ s"The computing unit was evicted by the cluster ($shortReason). Consider recreating it."
}
}
+ private def crashLoopReason(restartCount: Int): String =
+ s"The computing unit is repeatedly crashing (restarted $restartCount times). Please " +
+ "terminate and recreate it, or contact an administrator."
+
+ private def recoveredOomWarning(restartCount: Int): String =
+ s"The last run was terminated because the computing unit ran out of memory (restarted " +
+ s"$restartCount times). Consider recreating the unit with a higher memory limit before " +
+ "running the same workload."
+
+ /**
+ * Pure (snapshot -> state, reason) mapping, mirroring the Kubernetes pod lifecycle. An absent
+ * pod stays Pending — exactly today's behavior — because the vanish reconciliation, not this
+ * mapping, is what retires units whose pods are gone.
+ *
+ * Note the restartPolicy-Always subtlety: an OOM-killed container restarts in place with the
+ * pod phase still "Running", so OOM kills and crash loops are read from the container-level
+ * fields, and a waiting-state failure takes precedence over the recovered-OOM warning.
+ */
+ private[util] def kubernetesStatusAndReason(
+ snapshotOpt: Option[PodStatusSnapshot]
+ ): (ComputingUnitState, Option[String]) =
+ snapshotOpt match {
+ case None => (Pending, None)
+ case Some(snapshot) =>
+ val phase = snapshot.phase.getOrElse("")
+ val imagePullFailed =
+ snapshot.containers.exists(_.waitingReason.exists(ImagePullWaitingReasons.contains))
+ val crashLooping = snapshot.containers.find(_.waitingReason.contains("CrashLoopBackOff"))
+ val oomKilled = snapshot.containers.find(_.lastTerminatedReason.contains("OOMKilled"))
+
+ if (snapshot.terminating)
+ (Terminating, None)
+ else if (phase == "Failed" && snapshot.podReason.contains("Evicted"))
+ (Failed, Some(evictedReason(snapshot.podMessage)))
+ else if (imagePullFailed)
+ (Failed, Some(ImagePullReason))
+ else if (crashLooping.isDefined) {
+ val container = crashLooping.get
+ if (container.lastTerminatedReason.contains("OOMKilled"))
+ (Failed, Some(CrashLoopOomReason))
+ else
+ (Failed, Some(crashLoopReason(container.restartCount)))
+ } else if (phase == "Failed")
+ (Failed, Some(GenericFailedReason))
+ else if (phase == "Unknown")
+ (Unknown, Some(UnknownStateReason))
+ else if (phase == "Pending" && snapshot.unschedulable)
+ (Pending, Some(UnschedulableReason))
+ else if (phase == "Running" && oomKilled.isDefined)
+ (Running, Some(recoveredOomWarning(oomKilled.get.restartCount)))
+ else if (phase == "Running")
+ (Running, None)
Review Comment:
Fixed in a4f24d0ce6ca4176baefc6bd74129279a04bb72a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "eugenegujing (via GitHub)" <gi...@apache.org>.
eugenegujing commented on code in PR #7944:
URL: https://github.com/apache/texera/pull/7944#discussion_r4109561004
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -1563,15 +1572,25 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
if (this.isConnecting) {
return { label: "Connecting", icon: "loading", disabled: true };
}
+ // The socket may already be gone here, for a terminal unit or one that vanished mid-run.
+ // A kill on a dead socket is lost, so Stop is only enabled while it can be delivered.
if (this.isRunning) {
- return { label: "Stop", icon: "stop", disabled: false };
+ return { label: "Stop", icon: "stop", disabled: !this.workflowWebsocketService.isConnected };
}
Review Comment:
fixed in 1850e9614ac530c41209258d368f324e418a6a1a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 merged PR #7944:
URL: https://github.com/apache/texera/pull/7944
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #7944:
URL: https://github.com/apache/texera/pull/7944#discussion_r4101066078
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.html:
##########
@@ -209,19 +209,9 @@
[style.opacity]="0.7">
</texera-user-avatar>
<div class="computing-unit-name">
- <nz-badge
- [nzColor]="getBadgeColor(unit.status)"
- [nz-tooltip]="getUnitStatusTooltip(unit)"></nz-badge>
- <span
- *ngIf="editingNameOfUnit !== unit.computingUnit.cuid; else editableUnitName"
- nz-tooltip
- [nzTooltipTitle]="unit.computingUnit.uri">
Review Comment:
Did you intend to remove the tooltip on the unit name?
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -1563,15 +1572,25 @@ export class WorkflowFormComponent implements OnInit, OnDestroy {
if (this.isConnecting) {
return { label: "Connecting", icon: "loading", disabled: true };
}
+ // The socket may already be gone here, for a terminal unit or one that vanished mid-run.
+ // A kill on a dead socket is lost, so Stop is only enabled while it can be delivered.
if (this.isRunning) {
- return { label: "Stop", icon: "stop", disabled: false };
+ return { label: "Stop", icon: "stop", disabled: !this.workflowWebsocketService.isConnected };
}
Review Comment:
I think here if the unit dies mid-run, this shows a disabled Stop forever and the button never reaches Unavailable state. I suggest to move the `isConnected` condition in the if statement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "eugenegujing (via GitHub)" <gi...@apache.org>.
eugenegujing commented on code in PR #7944:
URL: https://github.com/apache/texera/pull/7944#discussion_r4099854477
##########
frontend/src/app/common/type/computing-unit-connection.interface.ts:
##########
@@ -24,5 +24,8 @@
export enum ComputingUnitState {
Running = "Running",
Pending = "Pending",
+ Failed = "Failed",
+ Unknown = "Unknown",
+ Terminating = "Terminating",
Review Comment:
Fixed in the latest commit, 830f26117.
##########
frontend/src/app/common/type/workflow-computing-unit.ts:
##########
@@ -46,7 +46,10 @@ export interface WorkflowComputingUnitMetrics {
export interface DashboardWorkflowComputingUnit {
computingUnit: WorkflowComputingUnit;
- status: "Running" | "Pending";
+ status: "Running" | "Pending" | "Failed" | "Unknown" | "Terminating";
+ // User-friendly explanation of a failing/degraded status for authorized viewers. The backend
+ // omits it for ordinary shared users, so the UI falls back to generic "unavailable" text.
+ statusReason?: string;
Review Comment:
Fixed in the latest commit, 830f26117.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #7944:
URL: https://github.com/apache/texera/pull/7944#issuecomment-5401445759
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/7944?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 91.51%. Comparing base ([`875aa72`](https://app.codecov.io/gh/apache/texera/commit/875aa72add3b7cb7ce23f024ce09ae1e983d39b7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d8707f2`](https://app.codecov.io/gh/apache/texera/commit/d8707f2559a12b0722f947ff41097b844496b525?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 47 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #7944 +/- ##
============================================
- Coverage 91.51% 91.51% -0.01%
+ Complexity 4476 4475 -1
============================================
Files 1171 1171
Lines 47199 47167 -32
Branches 5303 5299 -4
============================================
- Hits 43196 43164 -32
Misses 2344 2344
Partials 1659 1659
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/7944/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/7944/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.00% <ø> (ø)` | | |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/7944/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `98.62% <ø> (ø)` | | Carriedforward from [875aa72](https://app.codecov.io/gh/apache/texera/commit/875aa72add3b7cb7ce23f024ce09ae1e983d39b7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/7944/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `88.08% <ø> (ø)` | | Carriedforward from [875aa72](https://app.codecov.io/gh/apache/texera/commit/875aa72add3b7cb7ce23f024ce09ae1e983d39b7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/7944/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `72.05% <ø> (-1.63%)` | :arrow_down: | Carriedforward from [875aa72](https://app.codecov.io/gh/apache/texera/commit/875aa72add3b7cb7ce23f024ce09ae1e983d39b7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/7944/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `86.73% <ø> (ø)` | | |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/7944/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.90% <ø> (ø)` | | Carriedforward from [875aa72](https://app.codecov.io/gh/apache/texera/commit/875aa72add3b7cb7ce23f024ce09ae1e983d39b7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/7944/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `93.25% <ø> (-0.01%)` | :arrow_down: | Carriedforward from [875aa72](https://app.codecov.io/gh/apache/texera/commit/875aa72add3b7cb7ce23f024ce09ae1e983d39b7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/7944/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.13% <ø> (ø)` | | |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/7944/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `97.57% <ø> (ø)` | | Carriedforward from [875aa72](https://app.codecov.io/gh/apache/texera/commit/875aa72add3b7cb7ce23f024ce09ae1e983d39b7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/7944/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.19% <ø> (ø)` | | Carriedforward from [875aa72](https://app.codecov.io/gh/apache/texera/commit/875aa72add3b7cb7ce23f024ce09ae1e983d39b7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/7944?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7944:
URL: https://github.com/apache/texera/pull/7944#issuecomment-5401407318
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@Neilk1021`, `@kunwp1`, `@aglinxinyuan`
You can notify them by mentioning `@Neilk1021`, `@kunwp1`, `@aglinxinyuan` in a comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7944:
URL: https://github.com/apache/texera/pull/7944#discussion_r4068937652
##########
frontend/src/app/common/type/computing-unit-connection.interface.ts:
##########
@@ -24,5 +24,8 @@
export enum ComputingUnitState {
Running = "Running",
Pending = "Pending",
+ Failed = "Failed",
+ Unknown = "Unknown",
+ Terminating = "Terminating",
Review Comment:
These enum values are now emitted to every status consumer, but only `MenuComponent` was updated for them. `WorkflowFormComponent` still treats any selected non-`NoComputingUnit` state as `isConnecting` only when the socket is down, so `Failed`/`Unknown`/`Terminating` shows `Connecting` while disconnected and falls through to an enabled `Run` when connected; `OperatorMenu` and TimeTravel also call `ExecuteWorkflowService` directly. This lets users start or replay work on a unit the PR marks unavailable. Add a shared terminal-state guard in `ExecuteWorkflowService` and matching form labels/tests, preserving invalid/empty precedence.
##########
frontend/src/app/common/type/workflow-computing-unit.ts:
##########
@@ -46,7 +46,10 @@ export interface WorkflowComputingUnitMetrics {
export interface DashboardWorkflowComputingUnit {
computingUnit: WorkflowComputingUnit;
- status: "Running" | "Pending";
+ status: "Running" | "Pending" | "Failed" | "Unknown" | "Terminating";
+ // User-friendly explanation of a failing/degraded status for authorized viewers. The backend
+ // omits it for ordinary shared users, so the UI falls back to generic "unavailable" text.
+ statusReason?: string;
Review Comment:
The backend `Option[String]` field is documented and implemented as serializing to `null` for unauthorized/no-reason rows, but this DTO only permits `string` or an omitted property. Model the actual wire contract (including `null`) so consumers cannot rely on an impossible type.
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.html:
##########
@@ -198,7 +198,7 @@
'unit-selected': isSelectedUnit(unit),
'unit-connecting': unit.status === 'Pending',
}"
- [nz-tooltip]="cannotSelectUnit(unit) ? getUnitStatusTooltip(unit) + '. Cannot select.' : ''"
+ [nz-tooltip]="getRowTooltip(unit)"
Review Comment:
This changes visible badge, tooltip, and run-button behavior, but the PR description still contains its TODO for the required before/after capture and only provides standalone screenshots. The repository frontend review rule in `AGENTS.md:212-215` requires a side-by-side before/after screenshot or GIF for visible UI changes (and manual interaction steps for this flow); please attach that evidence, including the Run button, before approval.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "eugenegujing (via GitHub)" <gi...@apache.org>.
eugenegujing commented on code in PR #7944:
URL: https://github.com/apache/texera/pull/7944#discussion_r3877431424
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts:
##########
@@ -580,6 +580,26 @@ export class ComputingUnitSelectionComponent implements OnInit {
return getComputingUnitStatusTooltip(unit);
}
+ /**
+ * Row tooltip for a unit that cannot be selected. The status tooltip may already
+ * end with a period (e.g. an owner-facing statusReason), so trim it before
+ * appending the sentence to avoid a doubled dot.
+ */
+ getCannotSelectTooltip(unit: DashboardWorkflowComputingUnit): string {
+ return `${this.getUnitStatusTooltip(unit).replace(/\.$/, "")}. Cannot select.`;
+ }
+
+ /**
+ * The tooltip for a dropdown row: the status/reason, plus the cannot-select
+ * sentence when the unit is not selectable. The badge and name inside the row
+ * deliberately carry no tooltip of their own, so hovering the row body shows a
+ * single bubble. (The action icons keep their own tooltips, which stack on top
+ * of the row's while hovered — same as main's behavior on non-selectable rows.)
+ */
+ getRowTooltip(unit: DashboardWorkflowComputingUnit): string {
+ return this.cannotSelectUnit(unit) ? this.getCannotSelectTooltip(unit) : this.getUnitStatusTooltip(unit);
+ }
Review Comment:
Fixed in a4f24d0ce6ca4176baefc6bd74129279a04bb72a
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.scss:
##########
Review Comment:
Removed in a4f24d0ce6ca4176baefc6bd74129279a04bb72a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "eugenegujing (via GitHub)" <gi...@apache.org>.
eugenegujing commented on code in PR #7944:
URL: https://github.com/apache/texera/pull/7944#discussion_r3877428393
##########
frontend/src/app/common/service/computing-unit/computing-unit-status/computing-unit-status.service.ts:
##########
@@ -229,6 +229,10 @@ export class ComputingUnitStatusService implements OnDestroy {
return ComputingUnitState.Running;
case "Pending":
return ComputingUnitState.Pending;
+ case "Failed":
+ return ComputingUnitState.Failed;
+ case "Unknown":
+ return ComputingUnitState.Unknown;
Review Comment:
Added Terminating to ComputingUnitState, mapped it explicitly, made the run button show a disabled Shutting Down state and preserved the Pending fallback for genuinely unrecognized statuses in a4f24d0ce6ca4176baefc6bd74129279a04bb72a
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/AdminComputingUnitResource.scala:
##########
@@ -99,7 +99,7 @@ class AdminComputingUnitResource {
isOwner = unit.getUid.equals(user.getUid),
Review Comment:
Separated status-reason visibility from ownership so the admin list exposes statusReason for every row without incorrectly marking other users' units as owned, and ordinary shared users still receive no reason in a4f24d0ce6ca4176baefc6bd74129279a04bb72a
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/util/ComputingUnitHelpers.scala:
##########
@@ -55,42 +62,140 @@ object ComputingUnitHelpers {
}
def getComputingUnitStatus(unit: WorkflowComputingUnit): ComputingUnitState =
Review Comment:
Removed in a4f24d0ce6ca4176baefc6bd74129279a04bb72a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "eugenegujing (via GitHub)" <gi...@apache.org>.
eugenegujing commented on code in PR #7944:
URL: https://github.com/apache/texera/pull/7944#discussion_r4109553790
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.html:
##########
@@ -209,19 +209,9 @@
[style.opacity]="0.7">
</texera-user-avatar>
<div class="computing-unit-name">
- <nz-badge
- [nzColor]="getBadgeColor(unit.status)"
- [nz-tooltip]="getUnitStatusTooltip(unit)"></nz-badge>
- <span
- *ngIf="editingNameOfUnit !== unit.computingUnit.cuid; else editableUnitName"
- nz-tooltip
- [nzTooltipTitle]="unit.computingUnit.uri">
Review Comment:
Yes, that was intentional. URI tooltip and the row's status tooltip opened together and stacked two bubbles on hover. I don't think the URI information is really user-friendly so I replace it with only status information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #7944:
URL: https://github.com/apache/texera/pull/7944#discussion_r3868044439
##########
frontend/src/app/common/service/computing-unit/computing-unit-status/computing-unit-status.service.ts:
##########
@@ -229,6 +229,10 @@ export class ComputingUnitStatusService implements OnDestroy {
return ComputingUnitState.Running;
case "Pending":
return ComputingUnitState.Pending;
+ case "Failed":
+ return ComputingUnitState.Failed;
+ case "Unknown":
+ return ComputingUnitState.Unknown;
Review Comment:
Don't we need to also include "Terminating"? Did you intend the "Terminating" status to fall back to "Pending"?
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/AdminComputingUnitResource.scala:
##########
@@ -99,7 +99,7 @@ class AdminComputingUnitResource {
isOwner = unit.getUid.equals(user.getUid),
Review Comment:
I am planning to add an admin dashboard for computing unit. Since you added a string "contact an administrator", doesn't the admin also need to know the reason of the CU status?
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/util/ComputingUnitHelpers.scala:
##########
@@ -55,42 +62,140 @@ object ComputingUnitHelpers {
}
def getComputingUnitStatus(unit: WorkflowComputingUnit): ComputingUnitState =
- singleUnitStatus(unit, KubernetesClient)
+ getComputingUnitStatusWithReason(unit)._1
+
+ /** Single-unit status plus the owner-facing reason (see [[kubernetesStatusAndReason]]). */
+ def getComputingUnitStatusWithReason(
+ unit: WorkflowComputingUnit
+ ): (ComputingUnitState, Option[String]) =
+ singleUnitStatusAndReason(unit, KubernetesClient)
/**
* Single-unit status via a per-unit pod lookup (a targeted GET, cheaper than listing the whole
* namespace). The client is a by-name parameter — not the global singleton — so the kubernetes
* branch is unit-testable with a stub and the local/unknown branches never force the singleton;
- * the public overload binds the production [[KubernetesClient]]. (Metrics has no analogous seam:
+ * the public overloads bind the production [[KubernetesClient]]. (Metrics has no analogous seam:
* its per-unit lookup already fans out to the whole namespace and the bulk (unit, podMetrics)
* overload already covers the cpu/memory resolution, so nothing there is worth pinning.)
*/
- private[util] def singleUnitStatus(
+ private[util] def singleUnitStatusAndReason(
unit: WorkflowComputingUnit,
k8s: => KubernetesClient
- ): ComputingUnitState = {
+ ): (ComputingUnitState, Option[String]) = {
unit.getType match {
// Local CUs are always “running”
case WorkflowComputingUnitTypeEnum.local =>
- Running
+ (Running, None)
- // Kubernetes CUs – only explicit “Running” counts as running
+ // Kubernetes CUs – resolved from the pod's status snapshot
case WorkflowComputingUnitTypeEnum.kubernetes =>
- // Guard the pod status the same way the bulk getAllPodPhases does: a pod with no
- // status yet has a null getStatus, so map through Option to avoid an NPE.
val client = k8s
- val phaseOpt = client
- .getPodByName(client.generatePodName(unit.getCuid))
- .flatMap(pod => Option(pod.getStatus).map(_.getPhase))
-
- if (phaseOpt.contains("Running")) Running else Pending
+ kubernetesStatusAndReason(
+ client
+ .getPodByName(client.generatePodName(unit.getCuid))
+ .map(PodStatusSnapshot.fromPod)
+ )
// Any other (unknown) type is treated as pending
case _ =>
- Pending
+ (Pending, None)
+ }
+ }
+
+ // Owner-facing wording for each failure mode. Deliberately actionable prose, never a raw
+ // Kubernetes dump; buildDashboardUnit withholds these from non-owners entirely.
+ private val ImagePullWaitingReasons = Set("ImagePullBackOff", "ErrImagePull", "InvalidImageName")
+ private val EvictedDiskReason =
+ "The computing unit was evicted because it ran out of local disk storage. Consider " +
+ "storing less data on the unit's local file system, or recreate it with more storage."
+ private val ImagePullReason =
+ "The computing unit's image could not be pulled. Please recreate the unit or contact " +
+ "an administrator."
+ private val CrashLoopOomReason =
+ "The computing unit keeps crashing because it runs out of memory. Please terminate it " +
+ "and recreate it with a higher memory limit."
+ private val GenericFailedReason =
+ "The computing unit stopped unexpectedly. Please terminate and recreate it, or contact " +
+ "an administrator."
+ private val UnknownStateReason =
+ "The state of the computing unit cannot be determined (its node may be unreachable)."
+ private val UnschedulableReason =
+ "The computing unit is waiting for cluster resources to become available."
+
+ private def evictedReason(podMessage: Option[String]): String = {
+ val mentionsDisk =
+ podMessage.exists { message =>
+ val lower = message.toLowerCase
+ lower.contains("ephemeral") || lower.contains("disk")
+ }
+ if (mentionsDisk) EvictedDiskReason
+ else {
+ // First sentence of the cluster's message, capped so the tooltip stays readable.
+ val shortReason = podMessage
+ .map(_.takeWhile(_ != '.').trim)
+ .filter(_.nonEmpty)
+ .map(sentence => if (sentence.length > 120) sentence.take(120).trim + "..." else sentence)
+ .getOrElse("Evicted")
+ s"The computing unit was evicted by the cluster ($shortReason). Consider recreating it."
}
}
+ private def crashLoopReason(restartCount: Int): String =
+ s"The computing unit is repeatedly crashing (restarted $restartCount times). Please " +
+ "terminate and recreate it, or contact an administrator."
+
+ private def recoveredOomWarning(restartCount: Int): String =
+ s"The last run was terminated because the computing unit ran out of memory (restarted " +
+ s"$restartCount times). Consider recreating the unit with a higher memory limit before " +
+ "running the same workload."
+
+ /**
+ * Pure (snapshot -> state, reason) mapping, mirroring the Kubernetes pod lifecycle. An absent
+ * pod stays Pending — exactly today's behavior — because the vanish reconciliation, not this
+ * mapping, is what retires units whose pods are gone.
+ *
+ * Note the restartPolicy-Always subtlety: an OOM-killed container restarts in place with the
+ * pod phase still "Running", so OOM kills and crash loops are read from the container-level
+ * fields, and a waiting-state failure takes precedence over the recovered-OOM warning.
+ */
+ private[util] def kubernetesStatusAndReason(
+ snapshotOpt: Option[PodStatusSnapshot]
+ ): (ComputingUnitState, Option[String]) =
+ snapshotOpt match {
+ case None => (Pending, None)
+ case Some(snapshot) =>
+ val phase = snapshot.phase.getOrElse("")
+ val imagePullFailed =
+ snapshot.containers.exists(_.waitingReason.exists(ImagePullWaitingReasons.contains))
+ val crashLooping = snapshot.containers.find(_.waitingReason.contains("CrashLoopBackOff"))
+ val oomKilled = snapshot.containers.find(_.lastTerminatedReason.contains("OOMKilled"))
+
+ if (snapshot.terminating)
+ (Terminating, None)
+ else if (phase == "Failed" && snapshot.podReason.contains("Evicted"))
+ (Failed, Some(evictedReason(snapshot.podMessage)))
+ else if (imagePullFailed)
+ (Failed, Some(ImagePullReason))
+ else if (crashLooping.isDefined) {
+ val container = crashLooping.get
+ if (container.lastTerminatedReason.contains("OOMKilled"))
+ (Failed, Some(CrashLoopOomReason))
+ else
+ (Failed, Some(crashLoopReason(container.restartCount)))
+ } else if (phase == "Failed")
+ (Failed, Some(GenericFailedReason))
+ else if (phase == "Unknown")
+ (Unknown, Some(UnknownStateReason))
+ else if (phase == "Pending" && snapshot.unschedulable)
+ (Pending, Some(UnschedulableReason))
+ else if (phase == "Running" && oomKilled.isDefined)
+ (Running, Some(recoveredOomWarning(oomKilled.get.restartCount)))
+ else if (phase == "Running")
+ (Running, None)
Review Comment:
Collapse this into one.
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.scss:
##########
Review Comment:
Feel free to remove these too.
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/util/ComputingUnitHelpers.scala:
##########
@@ -55,42 +62,140 @@ object ComputingUnitHelpers {
}
def getComputingUnitStatus(unit: WorkflowComputingUnit): ComputingUnitState =
Review Comment:
Can you remove this function? I think it's a dead code now.
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts:
##########
@@ -580,6 +580,26 @@ export class ComputingUnitSelectionComponent implements OnInit {
return getComputingUnitStatusTooltip(unit);
}
+ /**
+ * Row tooltip for a unit that cannot be selected. The status tooltip may already
+ * end with a period (e.g. an owner-facing statusReason), so trim it before
+ * appending the sentence to avoid a doubled dot.
+ */
+ getCannotSelectTooltip(unit: DashboardWorkflowComputingUnit): string {
+ return `${this.getUnitStatusTooltip(unit).replace(/\.$/, "")}. Cannot select.`;
+ }
+
+ /**
+ * The tooltip for a dropdown row: the status/reason, plus the cannot-select
+ * sentence when the unit is not selectable. The badge and name inside the row
+ * deliberately carry no tooltip of their own, so hovering the row body shows a
+ * single bubble. (The action icons keep their own tooltips, which stack on top
+ * of the row's while hovered — same as main's behavior on non-selectable rows.)
+ */
+ getRowTooltip(unit: DashboardWorkflowComputingUnit): string {
+ return this.cannotSelectUnit(unit) ? this.getCannotSelectTooltip(unit) : this.getUnitStatusTooltip(unit);
+ }
Review Comment:
I think it's just better to fold getCannotSelectTooltip into getRowTooltip and move it to `computing-unit.util.ts` because it doesn't actually touch the component state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "eugenegujing (via GitHub)" <gi...@apache.org>.
eugenegujing commented on code in PR #7944:
URL: https://github.com/apache/texera/pull/7944#discussion_r3877428828
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/util/ComputingUnitHelpers.scala:
##########
@@ -55,42 +62,140 @@ object ComputingUnitHelpers {
}
def getComputingUnitStatus(unit: WorkflowComputingUnit): ComputingUnitState =
Review Comment:
Removed in a4f24d0ce6ca4176baefc6bd74129279a04bb72a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "eugenegujing (via GitHub)" <gi...@apache.org>.
eugenegujing commented on code in PR #7944:
URL: https://github.com/apache/texera/pull/7944#discussion_r4099854569
##########
frontend/src/app/common/type/workflow-computing-unit.ts:
##########
@@ -46,7 +46,10 @@ export interface WorkflowComputingUnitMetrics {
export interface DashboardWorkflowComputingUnit {
computingUnit: WorkflowComputingUnit;
- status: "Running" | "Pending";
+ status: "Running" | "Pending" | "Failed" | "Unknown" | "Terminating";
+ // User-friendly explanation of a failing/degraded status for authorized viewers. The backend
+ // omits it for ordinary shared users, so the UI falls back to generic "unavailable" text.
+ statusReason?: string;
Review Comment:
Fixed in 830f26117.
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.html:
##########
@@ -198,7 +198,7 @@
'unit-selected': isSelectedUnit(unit),
'unit-connecting': unit.status === 'Pending',
}"
- [nz-tooltip]="cannotSelectUnit(unit) ? getUnitStatusTooltip(unit) + '. Cannot select.' : ''"
+ [nz-tooltip]="getRowTooltip(unit)"
Review Comment:
Fixed in 830f26117.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" [texera]
Posted by "eugenegujing (via GitHub)" <gi...@apache.org>.
eugenegujing commented on PR #7944:
URL: https://github.com/apache/texera/pull/7944#issuecomment-5401546881
/request-review @kunwp1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org