choo121600 commented on code in PR #64540:
URL: https://github.com/apache/airflow/pull/64540#discussion_r3026892833
##########
airflow-core/src/airflow/ui/tests/e2e/utils/test-helpers.ts:
##########
@@ -497,29 +497,40 @@ export async function apiDeleteDagRun(source:
RequestLike, dagId: string, runId:
* Delete a DAG run, logging (not throwing) unexpected errors.
* Use this in fixture teardown where cleanup must not abort the loop.
* 404 is already handled inside `apiDeleteDagRun`.
- * 409 (running state) is handled by force-failing the run first, then
retrying.
+ *
+ * Strategy: force-fail the run first so the server doesn't wait for
+ * running tasks during deletion, then delete with one retry on timeout.
*/
export async function safeCleanupDagRun(source: RequestLike, dagId: string,
runId: string): Promise<void> {
+ const request = getRequestContext(source);
+
try {
- await apiDeleteDagRun(source, dagId, runId);
- } catch (error) {
- const message = error instanceof Error ? error.message : String(error);
-
- // 409 = DAG run is still running — force-fail, then retry deletion.
- if (message.includes("409")) {
- try {
- await apiSetDagRunState(source, { dagId, runId, state: "failed" });
- await apiDeleteDagRun(source, dagId, runId);
- } catch (retryError) {
- console.warn(
- `[e2e cleanup] Retry failed for DAG run ${dagId}/${runId}:
${retryError instanceof Error ? retryError.message : String(retryError)}`,
- );
+ await request.patch(`${baseUrl}/api/v2/dags/${dagId}/dagRuns/${runId}`, {
+ data: { state: "failed" },
+ headers: { "Content-Type": "application/json" },
+ timeout: 10_000,
+ });
+ } catch {
+ // Run may already be terminal or deleted — ignore.
+ }
+
+ for (let attempt = 0; attempt < 2; attempt++) {
+ try {
+ await apiDeleteDagRun(source, dagId, runId);
+
+ return;
+ } catch (error) {
+ const message = error instanceof Error ? error.message : String(error);
+ const isTimeout = message.includes("Timeout");
+
+ if (isTimeout && attempt === 0) {
+ continue;
}
Review Comment:
Since Playwright doesn’t export a TimeoutError, checking for "Timeout" in
the message is the most practical approach.
Given this is only used for cleanup with a single retry, even false
positives are acceptable. The current implementation looks good to me :)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
