Bug Fix Issue #4603 Access problem (#4604)

* Solved the access error

Signed-off-by: osobky <omar_sobky@hotmail.com>

* use more conservative function

Signed-off-by: harupy <hkawamura0130@gmail.com>

* comment

Signed-off-by: harupy <hkawamura0130@gmail.com>

* add test

Signed-off-by: harupy <hkawamura0130@gmail.com>

* fix test

Signed-off-by: harupy <hkawamura0130@gmail.com>

* rename variables

Signed-off-by: harupy <hkawamura0130@gmail.com>

* remove unused imports

Signed-off-by: harupy <hkawamura0130@gmail.com>

* lint

Signed-off-by: harupy <hkawamura0130@gmail.com>

* update docstring

Signed-off-by: harupy <hkawamura0130@gmail.com>

Co-authored-by: harupy <hkawamura0130@gmail.com>

mlflow/projects/docker.py

@@ -15,6 +15,7 @@
 from mlflow.tracking.context.git_context import _get_git_commit
 from mlflow.utils import process, file_utils
 from mlflow.utils.mlflow_tags import MLFLOW_DOCKER_IMAGE_URI, MLFLOW_DOCKER_IMAGE_ID
+from mlflow.utils.file_utils import _handle_readonly_on_windows
 
 _logger = logging.getLogger(__name__)
 
@@ -114,7 +115,7 @@ def _create_docker_build_ctx(work_dir, dockerfile_contents):
             output_filename=result_path, source_dir=dst_path, archive_name=_PROJECT_TAR_ARCHIVE_NAME
         )
     finally:
-        shutil.rmtree(directory)
+        shutil.rmtree(directory, onerror=_handle_readonly_on_windows)
     return result_path
 
 

mlflow/utils/file_utils.py

@@ -7,6 +7,7 @@
 import sys
 import tarfile
 import tempfile
+import stat
 
 import urllib.parse
 import urllib.request
@@ -459,3 +460,26 @@ def download_file_using_http_uri(http_uri, download_path, chunk_size=100000000):
                 if not chunk:
                     break
                 output_file.write(chunk)
+
+
+def _handle_readonly_on_windows(func, path, exc_info):
+    """
+    This function should not be called directly but should be passed to `onerror` of
+    `shutil.rmtree` in order to reattempt the removal of a read-only file after making
+    it writable on Windows.
+
+    References:
+    - https://bugs.python.org/issue19643
+    - https://bugs.python.org/issue43657
+    """
+    exc_type, exc_value = exc_info[:2]
+    should_reattempt = (
+        os.name == "nt"
+        and func in (os.unlink, os.rmdir)
+        and issubclass(exc_type, PermissionError)
+        and exc_value.winerror == 5
+    )
+    if not should_reattempt:
+        raise exc_value
+    os.chmod(path, stat.S_IWRITE)
+    func(path)

tests/projects/test_docker_projects.py

@@ -1,7 +1,6 @@
 import os
 
 import pytest
-import posixpath  # pylint: disable=unused-import
 from unittest import mock
 
 from databricks_cli.configure.provider import DatabricksConfig

tests/utils/test_file_utils.py

@@ -6,9 +6,15 @@
 import shutil
 import pytest
 import tarfile
+import stat
 
 from mlflow.utils import file_utils
-from mlflow.utils.file_utils import get_parent_dir, _copy_file_or_tree, TempDir
+from mlflow.utils.file_utils import (
+    get_parent_dir,
+    _copy_file_or_tree,
+    TempDir,
+    _handle_readonly_on_windows,
+)
 from tests.projects.utils import TEST_PROJECT_DIR
 
 from tests.helper_functions import random_int, random_file, safe_edit_yaml
@@ -168,3 +174,23 @@ def test_dir_copy():
             f.write("testing")
         _copy_file_or_tree(dir_path, copy_path, "")
         assert filecmp.dircmp(dir_path, copy_path)
+
+
+@pytest.mark.skipif(os.name != "nt", reason="requires Windows")
+def test_handle_readonly_on_windows(tmpdir):
+    tmp_path = tmpdir.join("file").strpath
+    with open(tmp_path, "w"):
+        pass
+
+    # Make the file read-only
+    os.chmod(tmp_path, stat.S_IREAD | stat.S_IRGRP | stat.S_IROTH)
+    # Ensure the file can't be removed
+    with pytest.raises(PermissionError, match="Access is denied") as exc:
+        os.unlink(tmp_path)
+
+    _handle_readonly_on_windows(
+        os.unlink,
+        tmp_path,
+        (exc.type, exc.value, exc.traceback),
+    )
+    assert not os.path.exists(tmp_path)