-
Notifications
You must be signed in to change notification settings - Fork 0
1871: feat(tui): Show job failed status below the Jobs table #74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| The AI agents should never link to any issue or a pull request | ||
| in any GitHub repository in the code reviews! | ||
|
|
||
| The AI agents should not review AI agents' config files like CLAUDE.md or AGENTS.md! | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| The AI agents should never link to any issue or a pull request | ||
| in any GitHub repository in the code reviews! | ||
|
|
||
| The AI agents should not review AI agents' config files like CLAUDE.md or AGENTS.md! | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| The AI agents should never link to any issue or a pull request | ||
| in any GitHub repository in the code reviews! | ||
|
|
||
| The AI agents should not review AI agents' config files like CLAUDE.md or AGENTS.md! | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| The AI agents should never link to any issue or a pull request | ||
| in any GitHub repository in the code reviews! | ||
|
|
||
| The AI agents should not review AI agents' config files like CLAUDE.md or AGENTS.md! | ||
|
|
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,7 +25,8 @@ use std::collections::BTreeMap; | |||||||||||
| pub struct Job { | ||||||||||||
| pub job_id: String, | ||||||||||||
| pub job_name: String, | ||||||||||||
| pub status: String, // Running, Completed, Failed, Canceled | ||||||||||||
| pub status: String, // Running, Completed, Failed, Canceled | ||||||||||||
| pub job_status: String, // human-readable status/failure detail, e.g. "Failed: <reason>" | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Severity: medium 🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
Comment on lines
+28
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a serde default for
Proposed fix #[derive(Deserialize, Clone, Debug)]
pub struct Job {
pub job_id: String,
pub job_name: String,
pub status: String, // Running, Completed, Failed, Canceled
+ #[serde(default)]
pub job_status: String, // human-readable status/failure detail, e.g. "Failed: <reason>"
pub start_time: i64,📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
| pub start_time: i64, | ||||||||||||
| pub end_time: i64, | ||||||||||||
| pub num_stages: usize, | ||||||||||||
|
|
@@ -440,6 +441,7 @@ mod tests { | |||||||||||
| job_id: id.to_string(), | ||||||||||||
| job_name: name.to_string(), | ||||||||||||
| status: status.to_string(), | ||||||||||||
| job_status: status.to_string(), | ||||||||||||
| start_time, | ||||||||||||
| end_time, | ||||||||||||
| num_stages, | ||||||||||||
|
|
@@ -681,6 +683,23 @@ mod tests { | |||||||||||
| assert_eq!(job.job_id, "j2"); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| #[test] | ||||||||||||
| fn job_status_carries_failure_detail_independent_of_status() { | ||||||||||||
| let job = Job { | ||||||||||||
| job_id: "j1".to_string(), | ||||||||||||
| job_name: "Job One".to_string(), | ||||||||||||
| status: "Failed".to_string(), | ||||||||||||
| job_status: "Failed: division by zero".to_string(), | ||||||||||||
| start_time: 1, | ||||||||||||
| end_time: 2, | ||||||||||||
| num_stages: 1, | ||||||||||||
| completed_stages: 0, | ||||||||||||
| percent_complete: 0, | ||||||||||||
| }; | ||||||||||||
| assert_eq!(job.status, "Failed"); | ||||||||||||
| assert_eq!(job.job_status, "Failed: division by zero"); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| #[test] | ||||||||||||
| fn selected_job_filters_by_search_term_on_id() { | ||||||||||||
| let jobs = vec![ | ||||||||||||
|
|
||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -49,7 +49,7 @@ use ratatui::{ | |||||||||||||||||||||||||||||||||||||||||||||
| style::Style, | ||||||||||||||||||||||||||||||||||||||||||||||
| text::Text, | ||||||||||||||||||||||||||||||||||||||||||||||
| widgets::{ | ||||||||||||||||||||||||||||||||||||||||||||||
| Block, Borders, Cell, HighlightSpacing, Paragraph, Row, Table, TableState, | ||||||||||||||||||||||||||||||||||||||||||||||
| Block, Borders, Cell, HighlightSpacing, Paragraph, Row, Table, TableState, Wrap, | ||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -200,24 +200,65 @@ pub fn render_jobs(f: &mut Frame, area: Rect, app: &App) { | |||||||||||||||||||||||||||||||||||||||||||||
| app.jobs_data.sort_jobs(&mut sorted_jobs); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if !sorted_jobs.is_empty() { | ||||||||||||||||||||||||||||||||||||||||||||||
| let selected_job = app | ||||||||||||||||||||||||||||||||||||||||||||||
| .jobs_data | ||||||||||||||||||||||||||||||||||||||||||||||
| .table_state | ||||||||||||||||||||||||||||||||||||||||||||||
| .selected() | ||||||||||||||||||||||||||||||||||||||||||||||
| .and_then(|idx| sorted_jobs.get(idx).copied()); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| let (table_area, failed_status_area) = | ||||||||||||||||||||||||||||||||||||||||||||||
| split_area_for_table_and_failure(selected_job, rects[1]); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| let mut scroll_state = app.jobs_data.scrollbar_state; | ||||||||||||||||||||||||||||||||||||||||||||||
| let mut table_state = app.jobs_data.table_state; | ||||||||||||||||||||||||||||||||||||||||||||||
| let table_area = vertical_scrollbar::split_area(rects[1]); | ||||||||||||||||||||||||||||||||||||||||||||||
| let [table_area, scrollbar_area] = vertical_scrollbar::split_area(table_area); | ||||||||||||||||||||||||||||||||||||||||||||||
| render_jobs_table( | ||||||||||||||||||||||||||||||||||||||||||||||
| f, | ||||||||||||||||||||||||||||||||||||||||||||||
| table_area[0], | ||||||||||||||||||||||||||||||||||||||||||||||
| table_area, | ||||||||||||||||||||||||||||||||||||||||||||||
| &sorted_jobs, | ||||||||||||||||||||||||||||||||||||||||||||||
| &mut table_state, | ||||||||||||||||||||||||||||||||||||||||||||||
| &app.jobs_data.sort_column, | ||||||||||||||||||||||||||||||||||||||||||||||
| &app.jobs_data.sort_order, | ||||||||||||||||||||||||||||||||||||||||||||||
| app, | ||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||
| render_scrollbar(f, table_area[1], &mut scroll_state); | ||||||||||||||||||||||||||||||||||||||||||||||
| render_scrollbar(f, scrollbar_area, &mut scroll_state); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if let Some((area, job)) = failed_status_area { | ||||||||||||||||||||||||||||||||||||||||||||||
| render_job_failure_reason(f, area, job, app); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||
| render_no_jobs(f, rects[1], app.theme.text_info); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| fn split_area_for_table_and_failure( | ||||||||||||||||||||||||||||||||||||||||||||||
| selected_job: Option<&Job>, | ||||||||||||||||||||||||||||||||||||||||||||||
| area: Rect, | ||||||||||||||||||||||||||||||||||||||||||||||
| ) -> (Rect, Option<(Rect, &Job)>) { | ||||||||||||||||||||||||||||||||||||||||||||||
| match selected_job { | ||||||||||||||||||||||||||||||||||||||||||||||
| Some(job) if job.status == "Failed" => { | ||||||||||||||||||||||||||||||||||||||||||||||
| let areas = Layout::vertical([ | ||||||||||||||||||||||||||||||||||||||||||||||
| Constraint::Min(5), // Table | ||||||||||||||||||||||||||||||||||||||||||||||
| Constraint::Length(5), // Failure reason | ||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||||||||||||||||||||||||||||||||||
| ]) | ||||||||||||||||||||||||||||||||||||||||||||||
| .split(area); | ||||||||||||||||||||||||||||||||||||||||||||||
| (areas[0], Some((areas[1], job))) | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| _ => (area, None), | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+238
to
+248
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the terminal window is resized to a very small height, splitting the area with a fixed constraint of
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| fn render_job_failure_reason(f: &mut Frame, area: Rect, job: &Job, app: &App) { | ||||||||||||||||||||||||||||||||||||||||||||||
| let block = Block::default() | ||||||||||||||||||||||||||||||||||||||||||||||
| .borders(Borders::all()) | ||||||||||||||||||||||||||||||||||||||||||||||
| .style(app.theme.text_error); | ||||||||||||||||||||||||||||||||||||||||||||||
| let paragraph = Paragraph::new(job.job_status.as_str()) | ||||||||||||||||||||||||||||||||||||||||||||||
| .style(app.theme.text_error) | ||||||||||||||||||||||||||||||||||||||||||||||
| .wrap(Wrap { trim: true }) | ||||||||||||||||||||||||||||||||||||||||||||||
| .block(block); | ||||||||||||||||||||||||||||||||||||||||||||||
| f.render_widget(paragraph, area); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| fn render_no_jobs(f: &mut Frame, area: Rect, style: Style) { | ||||||||||||||||||||||||||||||||||||||||||||||
| let block = Block::default().borders(Borders::all()); | ||||||||||||||||||||||||||||||||||||||||||||||
| let paragraph = Paragraph::new("No registered jobs in the scheduler!") | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To ensure backward compatibility and robustness against older scheduler versions or cases where the
job_statusfield is omitted from the API response, it is highly recommended to mark this field with#[serde(default)]. This prevents deserialization failures when the field is missing.