Copilot commented on code in PR #64753: URL: https://github.com/apache/airflow/pull/64753#discussion_r3066493281
########## dev/breeze/tests/utils/test_environment_context.py: ########## @@ -0,0 +1,227 @@ +# 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. +""" +Tests for environment context detection. + +Tests the ability to detect execution environment (WSL, Docker, etc.) +and recommend appropriate commands for Breeze workflows. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +from airflow_breeze.utils.environment_context import ( + EnvironmentContext, + _detect_wsl, + _is_docker_available, + detect_environment_context, +) + + +class TestDetectWSL: + """Tests for WSL detection.""" + + def test_detect_wsl_via_env_var(self, monkeypatch): + """Test WSL detection via WSL_DISTRO_NAME environment variable.""" + monkeypatch.setenv("WSL_DISTRO_NAME", "Ubuntu") + assert _detect_wsl() is True + + def test_detect_wsl_not_present_env_var(self, monkeypatch): + """Test when WSL_DISTRO_NAME is not set but on non-WSL system.""" + monkeypatch.delenv("WSL_DISTRO_NAME", raising=False) + + # Mock all detection methods to return non-WSL values + with patch("builtins.open", side_effect=OSError): # /proc/version not available + with patch("os.uname") as mock_uname: + mock_uname.return_value.release = "5.10.16.3-generic" # Non-microsoft kernel + result = _detect_wsl() + assert result is False + + def test_detect_wsl_via_proc_version(self, monkeypatch): + """Test WSL detection via /proc/version.""" + monkeypatch.delenv("WSL_DISTRO_NAME", raising=False) + + with patch("builtins.open", create=True) as mock_open: + mock_open.return_value.__enter__.return_value.read.return_value = ( + "Linux version 4.19.128-microsoft ([email protected]) " + "(gcc version 8.3.0 (GCC)) #1 SMP..." + ) + assert _detect_wsl() is True + + def test_detect_wsl_via_uname_release(self, monkeypatch): + """Test WSL detection via uname release.""" + monkeypatch.delenv("WSL_DISTRO_NAME", raising=False) + + with patch("builtins.open", side_effect=OSError): + with patch("os.uname") as mock_uname: + mock_uname.return_value.release = "4.19.128-microsoft-standard" + assert _detect_wsl() is True + + +class TestDockerAvailable: + """Tests for Docker availability detection.""" + + def test_docker_available_success(self): + """Test when Docker is available and running.""" + with patch("subprocess.run") as mock_run: + mock_result = MagicMock() + mock_result.returncode = 0 + mock_run.return_value = mock_result + Review Comment: Tests create `MagicMock()` instances without a `spec`/`autospec` (e.g. for the `subprocess.run` result). Using `spec`/`autospec` (or a real `subprocess.CompletedProcess`) helps catch attribute typos and keeps mocks aligned with the real API. ########## dev/breeze/tests/utils/test_environment_context.py: ########## @@ -0,0 +1,227 @@ +# 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 Review Comment: This file appears to be using CRLF (Windows-style) line endings. The repository's `.editorconfig` requires `end_of_line = lf`, so please normalize the file to LF to match project conventions. ########## dev/breeze/tests/utils/test_environment_context.py: ########## @@ -0,0 +1,227 @@ +# 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. +""" +Tests for environment context detection. + +Tests the ability to detect execution environment (WSL, Docker, etc.) +and recommend appropriate commands for Breeze workflows. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +from airflow_breeze.utils.environment_context import ( + EnvironmentContext, + _detect_wsl, + _is_docker_available, + detect_environment_context, +) + + +class TestDetectWSL: + """Tests for WSL detection.""" + + def test_detect_wsl_via_env_var(self, monkeypatch): + """Test WSL detection via WSL_DISTRO_NAME environment variable.""" + monkeypatch.setenv("WSL_DISTRO_NAME", "Ubuntu") + assert _detect_wsl() is True + + def test_detect_wsl_not_present_env_var(self, monkeypatch): + """Test when WSL_DISTRO_NAME is not set but on non-WSL system.""" + monkeypatch.delenv("WSL_DISTRO_NAME", raising=False) + + # Mock all detection methods to return non-WSL values + with patch("builtins.open", side_effect=OSError): # /proc/version not available Review Comment: There are whitespace-only/trailing spaces on otherwise blank lines (e.g. the blank line after `monkeypatch.delenv(...)`). This violates the repo's trailing-whitespace checks and should be cleaned up. ########## dev/breeze/tests/utils/test_environment_context.py: ########## @@ -0,0 +1,227 @@ +# 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. +""" +Tests for environment context detection. + +Tests the ability to detect execution environment (WSL, Docker, etc.) +and recommend appropriate commands for Breeze workflows. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + Review Comment: `Path` and `pytest` are imported but unused in this test module, which will fail linting (unused imports). Remove the unused imports or use them as intended. ```suggestion from unittest.mock import MagicMock, patch ``` ########## dev/breeze/src/airflow_breeze/utils/environment_context.py: ########## @@ -0,0 +1,146 @@ +# 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 Review Comment: This module also appears to be using CRLF line endings; `.editorconfig` specifies `end_of_line = lf`. Please convert the file to LF to align with repository conventions and avoid noisy diffs/tooling issues. ########## dev/breeze/src/airflow_breeze/utils/environment_context.py: ########## @@ -0,0 +1,146 @@ +# 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. +""" +Environment context detection helper for Breeze AI workflows. + +This module provides utilities to detect the execution environment (WSL, Docker, etc.) +and recommend appropriate commands for AI-assisted Breeze workflows. +""" + +from __future__ import annotations + +import os +import subprocess +from dataclasses import dataclass + + +@dataclass +class EnvironmentContext: + """Information about the current execution environment.""" + + is_wsl: bool + """True if running under Windows Subsystem for Linux (WSL).""" + + inside_container: bool + """True if running inside a Docker container.""" + + docker_available: bool + """True if Docker is available and accessible.""" + + recommended_command: str + """Recommended command for the current environment (e.g., 'breeze shell' or 'pytest').""" + + +def _detect_wsl() -> bool: + """ + Detect whether we are running under Windows Subsystem for Linux (WSL). + + Uses multiple detection methods: + 1. Check WSL_DISTRO_NAME environment variable (WSL 2 specific) + 2. Check /proc/version for Microsoft/WSL markers (WSL 1 & 2) + 3. Check uname release for microsoft marker (WSL 2) + + Returns: + True if running under WSL, False otherwise. + """ + # Method 1: Environment variable (WSL 2 specific) + if "WSL_DISTRO_NAME" in os.environ: + return True + Review Comment: WSL detection in `_detect_wsl()` duplicates existing logic in `airflow_breeze/commands/developer_commands.py:is_wsl()` and uses a different implementation. To avoid divergence/bugs over time, consider reusing a shared helper (or moving WSL detection into a common utils module and importing it from both places). ########## dev/breeze/src/airflow_breeze/utils/environment_context.py: ########## @@ -0,0 +1,146 @@ +# 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. +""" +Environment context detection helper for Breeze AI workflows. + +This module provides utilities to detect the execution environment (WSL, Docker, etc.) +and recommend appropriate commands for AI-assisted Breeze workflows. +""" + +from __future__ import annotations + +import os +import subprocess +from dataclasses import dataclass + + +@dataclass +class EnvironmentContext: + """Information about the current execution environment.""" + + is_wsl: bool + """True if running under Windows Subsystem for Linux (WSL).""" + + inside_container: bool + """True if running inside a Docker container.""" + + docker_available: bool + """True if Docker is available and accessible.""" + + recommended_command: str + """Recommended command for the current environment (e.g., 'breeze shell' or 'pytest').""" + + +def _detect_wsl() -> bool: + """ + Detect whether we are running under Windows Subsystem for Linux (WSL). + + Uses multiple detection methods: + 1. Check WSL_DISTRO_NAME environment variable (WSL 2 specific) + 2. Check /proc/version for Microsoft/WSL markers (WSL 1 & 2) + 3. Check uname release for microsoft marker (WSL 2) + + Returns: + True if running under WSL, False otherwise. + """ + # Method 1: Environment variable (WSL 2 specific) + if "WSL_DISTRO_NAME" in os.environ: + return True + + # Method 2: Check /proc/version (WSL 1 & 2) + try: + with open("/proc/version", encoding="utf-8") as version_file: + version = version_file.read() + if "Microsoft" in version or "WSL" in version: Review Comment: `/proc/version` detection is currently case-sensitive (`"Microsoft"` / `"WSL"`). Elsewhere in Breeze (e.g. `developer_commands.is_wsl`) the content is normalized with `.lower()` before checking, which is more robust. Consider lowercasing the file contents and checking for `"microsoft"`/`"wsl"` to avoid false negatives. ```suggestion version = version_file.read().lower() if "microsoft" in version or "wsl" in version: ``` ########## dev/breeze/src/airflow_breeze/utils/environment_context.py: ########## @@ -0,0 +1,146 @@ +# 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. +""" +Environment context detection helper for Breeze AI workflows. + +This module provides utilities to detect the execution environment (WSL, Docker, etc.) +and recommend appropriate commands for AI-assisted Breeze workflows. +""" + +from __future__ import annotations + +import os +import subprocess +from dataclasses import dataclass + + +@dataclass +class EnvironmentContext: + """Information about the current execution environment.""" + + is_wsl: bool + """True if running under Windows Subsystem for Linux (WSL).""" + + inside_container: bool + """True if running inside a Docker container.""" + + docker_available: bool + """True if Docker is available and accessible.""" + + recommended_command: str + """Recommended command for the current environment (e.g., 'breeze shell' or 'pytest').""" + + +def _detect_wsl() -> bool: + """ + Detect whether we are running under Windows Subsystem for Linux (WSL). + + Uses multiple detection methods: + 1. Check WSL_DISTRO_NAME environment variable (WSL 2 specific) + 2. Check /proc/version for Microsoft/WSL markers (WSL 1 & 2) + 3. Check uname release for microsoft marker (WSL 2) + + Returns: + True if running under WSL, False otherwise. + """ + # Method 1: Environment variable (WSL 2 specific) + if "WSL_DISTRO_NAME" in os.environ: + return True + + # Method 2: Check /proc/version (WSL 1 & 2) + try: + with open("/proc/version", encoding="utf-8") as version_file: + version = version_file.read() + if "Microsoft" in version or "WSL" in version: + return True + except OSError: + # /proc may not be available on non-Linux systems + pass + + # Method 3: Check uname release (WSL 2) + try: + release = os.uname().release + if "microsoft" in release.lower(): + return True + except (AttributeError, OSError): + pass + + return False + + +def _is_docker_available() -> bool: + """ + Check if Docker is available and accessible. + + Runs 'docker info' to verify Docker daemon is running. + + Returns: + True if Docker is available, False otherwise. + """ + try: + result = subprocess.run( + ["docker", "info"], + capture_output=True, + timeout=5, + check=False, + ) + return result.returncode == 0 + except (FileNotFoundError, subprocess.TimeoutExpired): + return False + + +def detect_environment_context() -> EnvironmentContext: + """ + Detect the current execution environment and return context information. + + Detects: + - Whether running in WSL + - Whether running inside a Docker container + - Whether Docker is available + - Recommended command based on environment + + Returns: + EnvironmentContext: Information about the current environment. + + Example: + >>> context = detect_environment_context() + >>> print(f"Running in container: {context.inside_container}") + >>> print(f"Recommended: {context.recommended_command}") + """ + # Check if inside Docker container + inside_container = os.path.exists("/.dockerenv") + + # Detect WSL + is_wsl_env = _detect_wsl() + + # Check Docker availability + docker_available = _is_docker_available() Review Comment: `detect_environment_context()` always runs `_is_docker_available()` (which shells out to `docker info` with a 5s timeout) even when `inside_container` is `True` and the result doesn't affect `recommended_command`. Consider skipping the Docker availability check when running inside the container, or making the availability probe opt-in, to keep context detection fast and predictable. ```suggestion # Check Docker availability only when running outside the container. docker_available = False if inside_container else _is_docker_available() ``` -- 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]
