Skip to content

Conversation

@Dev-iL
Copy link
Collaborator

@Dev-iL Dev-iL commented Dec 24, 2025

closes: #45172

Verified on linux via the unit tests on on win11 by running the script below:

# test_windows_path_fix.py
"""
Test script to verify the path normalization fix for Windows.
This tests the core logic without requiring a full Airflow installation.
"""
from pathlib import Path


def test_path_normalization():
    """
    Test that the fix correctly normalizes paths with mixed separators.
    
    This replicates the fix in airflow/dag_processing/dagbag.py:
        normalized_filepath = Path(filepath).as_posix()
        normalized_dags_folder = Path(settings.DAGS_FOLDER).as_posix()
        relative_file = normalized_filepath.replace(normalized_dags_folder, "")
    """
    test_cases = [
        # (dags_folder, filepath, expected_relative)
        # Windows backslash paths
        ("C:\\Users\\user\\dags", "C:\\Users\\user\\dags\\my_dag.py", "/my_dag.py"),
        # Mixed separators (the bug scenario from the issue)
        ("C:/Users/user/dags", "C:\\Users\\user\\dags\\my_dag.py", "/my_dag.py"),
        ("C:\\Users\\user/dags", "C:/Users/user/dags/my_dag.py", "/my_dag.py"),
        # Nested paths with mixed separators
        ("C:/Users/user/dags", "C:\\Users\\user\\dags\\subdir\\my_dag.py", "/subdir/my_dag.py"),
        # Forward slashes only (should still work)
        ("C:/Users/user/dags", "C:/Users/user/dags/my_dag.py", "/my_dag.py"),
    ]
    
    print("Testing path normalization fix...")
    print("-" * 60)
    
    all_passed = True
    for dags_folder, filepath, expected in test_cases:
        # This is the fix from dagbag.py
        normalized_filepath = Path(filepath).as_posix()
        normalized_dags_folder = Path(dags_folder).as_posix()
        relative_file = normalized_filepath.replace(normalized_dags_folder, "")
        
        passed = relative_file == expected
        status = "✓ PASS" if passed else "✗ FAIL"
        
        print(f"\n{status}")
        print(f"  DAGS_FOLDER: {dags_folder}")
        print(f"  filepath:    {filepath}")
        print(f"  Result:      {relative_file}")
        if not passed:
            print(f"  Expected:    {expected}")
            all_passed = False
    
    print("\n" + "-" * 60)
    
    # Also demonstrate what the OLD buggy code would have produced
    print("\nDemonstrating the OLD buggy behavior (without fix):")
    dags_folder = "C:/Users/user/dags"  # Forward slashes
    filepath = "C:\\Users\\user\\dags\\my_dag.py"  # Backslashes
    
    # Old code: simple string replace without normalization
    old_result = filepath.replace(dags_folder, "")
    print(f"  DAGS_FOLDER: {dags_folder}")
    print(f"  filepath:    {filepath}")
    print(f"  Old result:  {old_result}  <- BUG: absolute path returned!")
    
    # New code: normalize first
    new_result = Path(filepath).as_posix().replace(Path(dags_folder).as_posix(), "")
    print(f"  New result:  {new_result}  <- FIXED: relative path returned!")
    
    print("\n" + "=" * 60)
    if all_passed:
        print("✅ All tests passed! The fix works correctly on Windows.")
    else:
        print("❌ Some tests failed.")
    
    return all_passed


if __name__ == "__main__":
    success = test_path_normalization()
    exit(0 if success else 1)

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@Dev-iL Dev-iL force-pushed the 2512/dagbag_abspath_win branch from 91cc849 to e3beab2 Compare December 26, 2025 07:10
@Dev-iL Dev-iL changed the title Fix dagbag producing absolute paths on Windows DagBag: use Path.relative_to for consistent cross-platform behavior Dec 26, 2025
@Dev-iL Dev-iL force-pushed the 2512/dagbag_abspath_win branch from e3beab2 to 67705ca Compare December 26, 2025 08:21
@Dev-iL Dev-iL force-pushed the 2512/dagbag_abspath_win branch from 67705ca to 622ca80 Compare December 27, 2025 07:21
@Dev-iL Dev-iL force-pushed the 2512/dagbag_abspath_win branch from 622ca80 to 5811942 Compare December 27, 2025 10:13
@Dev-iL Dev-iL force-pushed the 2512/dagbag_abspath_win branch from 5811942 to 7edd999 Compare December 27, 2025 16:08
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Fantastic !

@potiuk potiuk force-pushed the 2512/dagbag_abspath_win branch from 7edd999 to d7ed22c Compare December 27, 2025 23:46
@potiuk
Copy link
Member

potiuk commented Dec 27, 2025

Rebased - the static error was caused by the breaing FastAPI change.

@potiuk potiuk merged commit 8c3b26e into apache:main Dec 27, 2025
33 checks passed
@Dev-iL Dev-iL deleted the 2512/dagbag_abspath_win branch December 28, 2025 07:15
chirodip98 pushed a commit to chirodip98/airflow-contrib that referenced this pull request Dec 28, 2025
amoghrajesh pushed a commit to astronomer/airflow that referenced this pull request Dec 29, 2025
Subham-KRLX pushed a commit to Subham-KRLX/airflow that referenced this pull request Jan 2, 2026
stegololz pushed a commit to stegololz/airflow that referenced this pull request Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Different behavior for DagBag.dagbag_stats.file on Linux vs Windows

2 participants