You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2026/10/05 17:31:45 UTC
[PR] Clean up deleted worktrees' Breeze resources automatically [airflow]
potiuk opened a new pull request, #74282:
URL: https://github.com/apache/airflow/pull/74282
Follow-up to #73836, which gave each git worktree its own Breeze Compose project and sweeps resources of deleted worktrees before Docker-backed commands. This builds on its label-based ownership to close the remaining gaps:
- **Resources are released soon after a worktree is deleted.** The first Docker-backed command in an isolated worktree starts a small detached watcher. It checks the directory every 30s and removes the worktree's containers, volumes and networks once the directory is gone. It uses only the standard library, imports everything at start-up (deleting the worktree can delete the venv it runs from), allows one instance per worktree via a file lock, and exits after an hour with no containers. It is not started in CI or with `--dry-run`. Until now, CPU and memory were only freed when some later Breeze command ran the sweep.
- **A shared Docker daemon no longer loses another machine's resources.** Resources are labelled `org.apache.airflow.breeze.host`, and only the host that created them treats them as stale. Resources without the label (created before this change) keep the old local-path check.
- **Same-named worktrees no longer collide.** The default project is `breeze-<dirname>-<6-char path hash>`.
- **Opt-out.** `breeze setup config --no-worktree-isolation` (stored in the main checkout's `.build`, so it applies to every worktree) or `BREEZE_WORKTREE_ISOLATION=true|false`. With isolation off, worktrees share the `breeze` project and their resources aren't tied to the worktree.
- **`breeze down` lists other worktrees** that still hold containers or volumes.
- **ADR 0019** records the reasoning for per-worktree isolation as a whole: label ownership, naming, what stays shared, cleanup triggers, safety rules, opt-out.
Note: the path hash renames existing worktree projects, so a worktree that already had a database volume starts with a fresh one once. `breeze down` in that worktree still removes the old project, because it's selected by its worktree label.
A second PR will scope the names that are still fixed: the `breeze-docs`/`breeze-db`/`breeze-constraints*`/`breeze-providers*` projects, the airflow-ctl and task-sdk test conftest projects, fixed `container_name`s, kind cluster names and the pycache volume.
Checks run locally:
- `uv run pytest` in `dev/breeze`: 1297 passed. Two port-binding tests in `test_run_utils.py` fail only because my local sandbox blocks binding.
- `prek run --from-ref main --stage pre-commit`, `prek run mypy-dev --all-files`: passed.
- `breeze setup regenerate-command-images --check-only` and `check-all-params-in-groups`: passed.
- The watcher was tested as a real standalone `python -I` process against a fake `docker` executable. It was not run against a live Docker daemon.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 5.5)
Generated-by: Claude Code (Opus 5.5) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Clean up deleted worktrees' Breeze resources automatically [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Clean up deleted worktrees' Breeze resources automatically [airflow]
Posted by "Andrushika (via GitHub)" <gi...@apache.org>.
Andrushika commented on code in PR #74282:
URL: https://github.com/apache/airflow/pull/74282#discussion_r4291514848
##########
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:
Nit: we might need to also check that the main repository's `.git` still exists here, like `get_main_git_dir_for_worktree` does.
Otherwise moving or renaming the main checkout makes every worktree look deleted, and the watcher removes their containers and volumes.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Clean up deleted worktrees' Breeze resources automatically [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #74282:
URL: https://github.com/apache/airflow/pull/74282
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Clean up deleted worktrees' Breeze resources automatically [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #74282:
URL: https://github.com/apache/airflow/pull/74282#discussion_r4291297662
##########
dev/breeze/src/airflow_breeze/utils/worktree_watcher.py:
##########
@@ -0,0 +1,148 @@
+# 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:
Review Comment:
Good catch, thanks for testing it for real. Done in [144edf1](https://github.com/apache/airflow/commit/144edf1dfd). `is_worktree_missing` now follows the `.git` → `gitdir` link, as you suggested, so it catches leftover bind-mount folders and a failed `git worktree remove`. Errors other than "not found" are passed up to the caller, so the resources are kept when the check itself fails. There are tests for each case: intact, worktree deleted, folders left behind, `gitdir` gone, and a relative `gitdir`.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
dev/breeze/src/airflow_breeze/utils/docker_command_utils.py:
##########
@@ -827,12 +867,21 @@ def remove_stale_worktree_containers() -> None:
console_print("[error]Unable to discover containers belonging to deleted worktrees.[/]")
return
for line in result.stdout.splitlines():
- container_id, _, worktree = line.partition("\t")
- if _worktree_is_missing(worktree):
+ container_id, worktree, host = (line.split("\t") + ["", ""])[:3]
+ # `docker ps` prints "<no value>" for containers created before the host label existed.
+ if _belongs_to_deleted_worktree(worktree, "" if host == "<no value>" else host):
console_print(f"Removing container {container_id} for deleted worktree {worktree}")
run_command(["docker", "rm", "--force", "--volumes", container_id], check=False)
+def _belongs_to_deleted_worktree(worktree: str, host: str) -> bool:
+ # Another host's paths cannot be checked here. Resources created before the host label existed
+ # have no host and keep the local-path check.
+ if host and host != get_host_id():
+ return False
+ return _worktree_is_missing(worktree)
Review Comment:
Agreed. In [144edf1](https://github.com/apache/airflow/commit/144edf1dfd), `_worktree_is_missing` now calls `worktree_watcher.is_worktree_missing`. It keeps only the relative-path guard and the warning when the worktree can't be checked, so the watcher and the sweep can no longer drift apart.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
dev/breeze/src/airflow_breeze/utils/path_utils.py:
##########
@@ -640,11 +641,49 @@ def cleanup_python_generated_files():
console_print("[info]Cleaned")
+WORKTREE_ISOLATION_ENV = "BREEZE_WORKTREE_ISOLATION"
+SUPPRESS_WORKTREE_ISOLATION_FILE = "suppress_worktree_isolation"
+
+
+def get_shared_build_cache_path() -> Path:
+ """Return the ``.build`` directory of the main checkout, shared by all of its worktrees."""
+ main_git_dir = get_main_git_dir_for_worktree()
+ return main_git_dir.parent / ".build" if main_git_dir else BUILD_CACHE_PATH
+
+
+def is_worktree_isolation_enabled() -> bool:
+ value = os.environ.get(WORKTREE_ISOLATION_ENV, "").strip().lower()
+ if value in ("true", "1", "yes", "on"):
+ return True
+ if value in ("false", "0", "no", "off"):
+ return False
+ return not (get_shared_build_cache_path() / SUPPRESS_WORKTREE_ISOLATION_FILE).exists()
+
+
+def get_isolated_worktree_path() -> Path | None:
+ """
+ Return the path of the linked worktree that owns this checkout's Docker resources.
+
+ ``None`` means the resources are shared: the main checkout, or a worktree with isolation disabled.
+ """
+ if get_main_git_dir_for_worktree() is None or not is_worktree_isolation_enabled():
+ return None
+ return AIRFLOW_ROOT_PATH.resolve()
+
+
def get_default_project_name() -> str:
- if get_main_git_dir_for_worktree() is None:
+ worktree = get_isolated_worktree_path()
+ if worktree is None:
return "breeze"
- name = re.sub(r"[^a-z0-9_-]", "-", AIRFLOW_ROOT_PATH.resolve().name.lower())
- return f"breeze-{name}"
+ name = re.sub(r"[^a-z0-9_-]", "-", worktree.name.lower())
+ # Worktrees with the same directory name in different locations or clones must not share a project.
+ path_hash = hashlib.sha256(str(worktree).encode()).hexdigest()[:6]
+ return f"breeze-{name}-{path_hash}"
+
+
+def get_host_id() -> str:
+ """Identify the machine whose filesystem decides whether a labelled worktree still exists."""
+ return socket.gethostname()
Review Comment:
Yes, that works better. Done in [144edf1](https://github.com/apache/airflow/commit/144edf1dfd). `get_host_id()` now creates a random id once and stores it in `~/.config/breeze/host-id`. The file is created with a link that fails if it already exists, so two Breeze runs starting at once still end up with the same id. If the home directory isn't writable (for example in a sandbox), it falls back to the host name. [05ea9a8](https://github.com/apache/airflow/commit/05ea9a8cbe) also treats resources labelled with the current host name as local. That covers resources created before this change or with the fallback, so they still get swept.
---
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Clean up deleted worktrees' Breeze resources automatically [airflow]
Posted by "Andrushika (via GitHub)" <gi...@apache.org>.
Andrushika commented on code in PR #74282:
URL: https://github.com/apache/airflow/pull/74282#discussion_r4205174599
##########
dev/breeze/src/airflow_breeze/utils/worktree_watcher.py:
##########
@@ -0,0 +1,148 @@
+# 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:
Review Comment:
On macOS with Docker Desktop, the worktree path does not go away while its container is running, so this check never fires in the case the watcher is for.
For example, with `breeze shell` running in a worktree, `rm -rf` of that worktree fails with "Permission denied" on `devel-common` and `providers-summary-docs` (the bind-mount sources), and both stay. `.git` is gone. The watcher kept polling for 2.5 minutes and the container kept running. After I stopped the container and removed the two directories, the watcher cleaned everything up within 20 seconds.
Checking the link to the main repository instead of the path would cover this. It also covers a failed `git worktree remove --force`, which leaves a `.git` file whose gitdir is gone:
```python
def is_worktree_missing(worktree: Path) -> bool:
try:
gitdir = (worktree / ".git").read_text().strip().removeprefix("gitdir:").strip()
(worktree / gitdir).stat()
except FileNotFoundError:
return True
except OSError:
return False
return False
```
##########
dev/breeze/src/airflow_breeze/utils/path_utils.py:
##########
@@ -640,11 +641,49 @@ def cleanup_python_generated_files():
console_print("[info]Cleaned")
+WORKTREE_ISOLATION_ENV = "BREEZE_WORKTREE_ISOLATION"
+SUPPRESS_WORKTREE_ISOLATION_FILE = "suppress_worktree_isolation"
+
+
+def get_shared_build_cache_path() -> Path:
+ """Return the ``.build`` directory of the main checkout, shared by all of its worktrees."""
+ main_git_dir = get_main_git_dir_for_worktree()
+ return main_git_dir.parent / ".build" if main_git_dir else BUILD_CACHE_PATH
+
+
+def is_worktree_isolation_enabled() -> bool:
+ value = os.environ.get(WORKTREE_ISOLATION_ENV, "").strip().lower()
+ if value in ("true", "1", "yes", "on"):
+ return True
+ if value in ("false", "0", "no", "off"):
+ return False
+ return not (get_shared_build_cache_path() / SUPPRESS_WORKTREE_ISOLATION_FILE).exists()
+
+
+def get_isolated_worktree_path() -> Path | None:
+ """
+ Return the path of the linked worktree that owns this checkout's Docker resources.
+
+ ``None`` means the resources are shared: the main checkout, or a worktree with isolation disabled.
+ """
+ if get_main_git_dir_for_worktree() is None or not is_worktree_isolation_enabled():
+ return None
+ return AIRFLOW_ROOT_PATH.resolve()
+
+
def get_default_project_name() -> str:
- if get_main_git_dir_for_worktree() is None:
+ worktree = get_isolated_worktree_path()
+ if worktree is None:
return "breeze"
- name = re.sub(r"[^a-z0-9_-]", "-", AIRFLOW_ROOT_PATH.resolve().name.lower())
- return f"breeze-{name}"
+ name = re.sub(r"[^a-z0-9_-]", "-", worktree.name.lower())
+ # Worktrees with the same directory name in different locations or clones must not share a project.
+ path_hash = hashlib.sha256(str(worktree).encode()).hexdigest()[:6]
+ return f"breeze-{name}-{path_hash}"
+
+
+def get_host_id() -> str:
+ """Identify the machine whose filesystem decides whether a labelled worktree still exists."""
+ return socket.gethostname()
Review Comment:
Minor: `socket.gethostname()` can change on macOS. If it does, the old resources look like another machine's, and the stale-worktree sweep skips them for good.
For example, when `scutil --get HostName` is not set (the default), the name follows the LocalHostName, and macOS adds a number to it when another device on the network already uses the name. Mine already has a `-2` suffix. After such a rename, resources of a deleted worktree stay until someone runs `breeze down --all-worktrees`.
Would a random id written once to a file under the user's home work here?
##########
dev/breeze/src/airflow_breeze/utils/docker_command_utils.py:
##########
@@ -827,12 +867,21 @@ def remove_stale_worktree_containers() -> None:
console_print("[error]Unable to discover containers belonging to deleted worktrees.[/]")
return
for line in result.stdout.splitlines():
- container_id, _, worktree = line.partition("\t")
- if _worktree_is_missing(worktree):
+ container_id, worktree, host = (line.split("\t") + ["", ""])[:3]
+ # `docker ps` prints "<no value>" for containers created before the host label existed.
+ if _belongs_to_deleted_worktree(worktree, "" if host == "<no value>" else host):
console_print(f"Removing container {container_id} for deleted worktree {worktree}")
run_command(["docker", "rm", "--force", "--volumes", container_id], check=False)
+def _belongs_to_deleted_worktree(worktree: str, host: str) -> bool:
+ # Another host's paths cannot be checked here. Resources created before the host label existed
+ # have no host and keep the local-path check.
+ if host and host != get_host_id():
+ return False
+ return _worktree_is_missing(worktree)
Review Comment:
The sweep here and the watcher now each have their own "is this worktree deleted" check, and they already differ a bit (this one skips relative paths and warns on `OSError`).
For example, a fix to the watcher's check for the leftover-directories case would not reach this sweep, which has the same problem.
This module already imports `worktree_watcher`, so `_worktree_is_missing` could call `worktree_watcher.is_worktree_missing` and keep only the relative-path guard and the warning here.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Clean up deleted worktrees' Breeze resources automatically [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on PR #74282:
URL: https://github.com/apache/airflow/pull/74282#issuecomment-6235622453
Super important, thank you Jarek!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Clean up deleted worktrees' Breeze resources automatically [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #74282:
URL: https://github.com/apache/airflow/pull/74282#issuecomment-6266362096
### Backport successfully created: v3-4-test
Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>✅</td>
<td>v3-4-test</td>
<td><a href="https://github.com/apache/airflow/pull/75093"><img src="https://img.shields.io/badge/PR-75093-blue" alt="PR Link"></a></td>
</tr>
</table>
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org