potiuk commented on code in PR #74282: URL: https://github.com/apache/airflow/pull/74282#discussion_r4313244264
########## dev/breeze/src/airflow_breeze/utils/worktree_watcher.py: ########## @@ -0,0 +1,160 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Detached watcher that removes the Docker resources of a deleted Breeze worktree. + +Breeze starts it as a separate process with ``python -I <this file>``. The watcher must keep working +after the worktree is deleted, and that deletion can take Breeze's own virtualenv with it, so this +module only uses the standard library, imports everything up front and never imports +``airflow_breeze``. +""" + +from __future__ import annotations + +import argparse +import contextlib +import fcntl +import subprocess +import time +from pathlib import Path +from typing import IO + +OWNER_LABEL = "org.apache.airflow.breeze=true" +WORKTREE_LABEL = "org.apache.airflow.breeze.worktree" +HOST_LABEL = "org.apache.airflow.breeze.host" +DOCKER_TIMEOUT_SECONDS = 120 + + +def try_lock(lock_file: Path) -> IO[str] | None: + """Take the watcher lock, or return ``None`` when another process holds it.""" + lock_file.parent.mkdir(parents=True, exist_ok=True) + handle = lock_file.open("a") + try: + fcntl.flock(handle, fcntl.LOCK_EX | fcntl.LOCK_NB) + except BlockingIOError: + handle.close() + return None + return handle + + +def is_watcher_running(lock_file: Path) -> bool: + handle = try_lock(lock_file) + if handle is None: + return True + handle.close() + return False + + +def is_worktree_missing(worktree: Path) -> bool: + """ + Tell whether the worktree is deleted, judged by its link to the main repository. + + Checking the worktree path alone is not enough: Docker Desktop on macOS keeps bind-mounted + directories of a running container when the worktree is deleted, and a failed + ``git worktree remove`` leaves a ``.git`` file whose ``gitdir`` no longer exists. + + Errors other than a missing file propagate, so that callers keep the resources. + """ + try: + gitdir = (worktree / ".git").read_text().strip().removeprefix("gitdir:").strip() + (worktree / gitdir).stat() Review Comment: Good point. Fixed in bc09956f6c: `is_worktree_missing` now treats a missing `gitdir` as a deleted worktree only while the main repository's `.git` still exists, as `get_main_git_dir_for_worktree` does. If the main checkout was moved or renamed, the worktrees count as present and their resources are kept. The stale-worktree sweep calls the same function, and there's a new test for a moved main checkout. --- Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting -- 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]
