Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ const toTooltipSummary = (

return {
child_states: null,
max_end_date: dayjs(segment.x[1]).toISOString(),
max_end_date: segment.end_when ?? dayjs(segment.x[1]).toISOString(),
min_start_date: segment.start_when ?? dayjs(segment.x[0]).toISOString(),
state: segment.state ?? null,
task_display_name: segment.y,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,65 @@ describe("transformGanttData", () => {
expect(result[2]?.state).toBe("success");
});

it("carries the task's actual start and end on every segment of the try", () => {
const result = transformGanttData({
allTries: [
{
end_date: "2024-03-14T10:05:00+00:00",
is_mapped: false,
queued_dttm: "2024-03-14T09:59:00+00:00",
scheduled_dttm: "2024-03-14T09:58:00+00:00",
start_date: "2024-03-14T10:00:00+00:00",
state: "success",
task_display_name: "task_1",
task_id: "task_1",
try_number: 1,
},
],
flatNodes: [{ depth: 0, id: "task_1", is_mapped: false, label: "task_1" }],
gridSummaries: [],
});

expect(result).toHaveLength(3);
// The scheduled, queued, and execution bars all report the task's real start_date/end_date so
// the tooltip is consistent no matter which segment is hovered (regression from #68174).
const expectedEnd = new Date("2024-03-14T10:05:00+00:00").toISOString();

for (const segment of result) {
expect(segment.start_when).toBe("2024-03-14T10:00:00+00:00");
expect(segment.end_when).toBe(expectedEnd);
}
});

it("uses the current time as end_when on every segment while the task is still running", () => {
const result = transformGanttData({
allTries: [
{
end_date: null,
is_mapped: false,
queued_dttm: "2024-03-14T09:59:00+00:00",
scheduled_dttm: null,
start_date: "2024-03-14T10:00:00+00:00",
state: "running",
task_display_name: "task_1",
task_id: "task_1",
try_number: 1,
},
],
flatNodes: [{ depth: 0, id: "task_1", is_mapped: false, label: "task_1" }],
gridSummaries: [],
});

// Queued + execution bars, both reporting the same (running) end so the tooltip is consistent.
expect(result.length).toBeGreaterThan(0);
const [firstEnd] = result.map((segment) => segment.end_when);

for (const segment of result) {
expect(segment.end_when).toBe(firstEnd);
expect(segment.end_when).not.toBeUndefined();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you tighten this to pin the actual value of end_when, not just that it's consistent across segments? With vi.useFakeTimers() + vi.setSystemTime(...) you can also assert end_when matches that fixed "now" — otherwise a regression returning a stale or zero timestamp would slip past.

Suggested change
}
it("uses the current time as end_when on every segment while the task is still running", () => {
vi.useFakeTimers();
vi.setSystemTime(new Date("2024-03-14T10:02:00+00:00"));
try {
const result = transformGanttData({
allTries: [
{
end_date: null,
is_mapped: false,
queued_dttm: "2024-03-14T09:59:00+00:00",
scheduled_dttm: null,
start_date: "2024-03-14T10:00:00+00:00",
state: "running",
task_display_name: "task_1",
task_id: "task_1",
try_number: 1,
},
],
flatNodes: [{ depth: 0, id: "task_1", is_mapped: false, label: "task_1" }],
gridSummaries: [],
});
const expectedEnd = new Date("2024-03-14T10:02:00+00:00").toISOString();
for (const segment of result) {
expect(segment.end_when).toBe(expectedEnd);
}
} finally {
vi.useRealTimers();
}
});

});

it("produces 2 segments when only queued_dttm is present", () => {
const result = transformGanttData({
allTries: [
Expand Down
20 changes: 13 additions & 7 deletions airflow-core/src/airflow/ui/src/layouts/Details/Gantt/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import { renderDuration } from "src/utils/datetimeUtils";
import { buildTaskInstanceUrl } from "src/utils/links";

export type GanttDataItem = {
/** Effective task end (end_date, or "now" while running) — consistent across all segments of the same try. */
end_when?: string | null;
isGroup?: boolean | null;
isMapped?: boolean | null;
/** Source try times for tooltips (matches TaskInstance `*_when` fields). */
Expand Down Expand Up @@ -135,13 +137,6 @@ export const transformGanttData = ({
const queuedMs = queuedDttm === null ? undefined : dayjs(queuedDttm).valueOf();
const scheduledMs = scheduledDttm === null ? undefined : dayjs(scheduledDttm).valueOf();

// Include scheduled/queued/start times in tooltip data whenever the timestamps exist.
const tryWhenForTooltip = {
...(scheduledMs === undefined ? {} : { scheduled_when: scheduledDttm }),
...(queuedMs === undefined ? {} : { queued_when: queuedDttm }),
...(startDate === null ? {} : { start_when: startDate }),
};

let endMs: number;

if (hasTaskRunning) {
Expand All @@ -152,6 +147,17 @@ export const transformGanttData = ({
endMs = dayjs(endDate).valueOf();
}

// Include scheduled/queued/start/end times in tooltip data whenever the timestamps exist.
// start_when/end_when are carried on every segment of a try so the tooltip reports the
// task's actual start and end on the scheduled and queued bars too, not just the
// execution bar's own bounds.
const tryWhenForTooltip = {
...(scheduledMs === undefined ? {} : { scheduled_when: scheduledDttm }),
...(queuedMs === undefined ? {} : { queued_when: queuedDttm }),
...(startDate === null ? {} : { start_when: startDate }),
...(startDate === null && !hasTaskRunning ? {} : { end_when: dayjs(endMs).toISOString() }),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two lines aren't symmetric with each other:

  • Gate: start_when skips only when startDate === null; end_when adds && !hasTaskRunning. The extra branch only fires for queued/scheduled tasks (isStatePending is true) with no start_date, where the tooltip would then show Start Date = the bar's own start (segment fallback) + End Date = Date.now(). Two different scales in the same tooltip.
  • Value: start_when keeps the raw API string ("2024-03-14T10:00:00+00:00"); end_when is re-serialized via dayjs(endMs).toISOString() to "...Z" with .000 millis. The test even pins this skew. Renders fine because dayjs reparses, but it's needless and it's the reason the tryWhenForTooltip block had to move below endMs.

Suggest collapsing both into a single symmetric form — derive an effectiveEndDate and gate on it like start_when does on startDate:

Suggested change
...(startDate === null && !hasTaskRunning ? {} : { end_when: dayjs(endMs).toISOString() }),
const effectiveEndDate = hasTaskRunning ? new Date().toISOString() : endDate;
// Include scheduled/queued/start/end times in tooltip data whenever the timestamps exist.
// start_when/end_when are carried on every segment of a try so the tooltip reports the
// task's actual start and end on the scheduled and queued bars too, not just the
// execution bar's own bounds.
const tryWhenForTooltip = {
...(scheduledMs === undefined ? {} : { scheduled_when: scheduledDttm }),
...(queuedMs === undefined ? {} : { queued_when: queuedDttm }),
...(startDate === null ? {} : { start_when: startDate }),
...(effectiveEndDate === null ? {} : { end_when: effectiveEndDate }),
};

Then the block doesn't need to live below endMs anymore (no ordering dependency) and finished tasks keep the API's raw end_date string. The test for the finished task can drop the new Date(...).toISOString() wrapping and assert against the raw "2024-03-14T10:05:00+00:00" like it does for start_when.

};

if (scheduledMs !== undefined) {
const scheduledEndMs =
queuedMs ?? startMs ?? (hasTaskRunning || tryRow.state === "scheduled" ? Date.now() : endMs);
Expand Down
Loading