feat(toolkit): use sha in resulting repo name to avoid race conditions

fal-ai · Jan 25, 2025 · bb0d042 · bb0d042
1 parent 306261d
commit bb0d042
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 7 deletions.
diff --git a/projects/fal/src/fal/toolkit/utils/download_utils.py b/projects/fal/src/fal/toolkit/utils/download_utils.py
@@ -389,7 +389,11 @@ def clone_repository(
         A Path object representing the full path to the cloned Git repository.
     """
     target_dir = target_dir or FAL_REPOSITORY_DIR  # type: ignore[assignment]
-    repo_name = repo_name or Path(https_url).stem
+
+    if repo_name is None:
+        repo_name = Path(https_url).stem
+        if commit_hash:
+            repo_name += f"-{commit_hash[:8]}"
 
     local_repo_path = Path(target_dir) / repo_name  # type: ignore[arg-type]
 

diff --git a/projects/fal/tests/integration_test.py b/projects/fal/tests/integration_test.py
@@ -355,6 +355,12 @@ def test_clone_repository(isolated_client, mock_fal_persistent_dirs):
     EXAMPLE_REPO_FIRST_COMMIT = "64b0a89c8391bd2cb3ca23cdeae01779e11aee05"
     EXAMPLE_REPO_SECOND_COMMIT = "34ecbca8cc7b64719d2a5c40dd3272f8d13bc1d2"
     expected_path = FAL_REPOSITORY_DIR / "isolate"
+    first_expected_path = (
+        FAL_REPOSITORY_DIR / f"isolate-{EXAMPLE_REPO_FIRST_COMMIT[:8]}"
+    )
+    second_expected_path = (
+        FAL_REPOSITORY_DIR / f"isolate-{EXAMPLE_REPO_SECOND_COMMIT[:8]}"
+    )
 
     @isolated_client()
     def clone_without_commit_hash():
@@ -376,7 +382,7 @@ def clone_with_commit_hash():
             EXAMPLE_REPO_URL, commit_hash=EXAMPLE_REPO_SECOND_COMMIT
         )
 
-        second_repo_hash = _get_git_revision_hash(repo_path)
+        second_repo_hash = _get_git_revision_hash(second_path)
 
         return first_path, first_repo_hash, second_path, second_repo_hash
 
@@ -387,8 +393,12 @@ def clone_with_commit_hash():
         second_repo_hash,
     ) = clone_with_commit_hash()
 
-    assert str(expected_path) == str(first_path), "Path should be the target location"
-    assert str(expected_path) == str(second_path), "Path should be the target location"
+    assert str(first_expected_path) == str(
+        first_path
+    ), "Path should be the target location"
+    assert str(second_expected_path) == str(
+        second_path
+    ), "Path should be the target location"
 
     assert (
         first_repo_hash == EXAMPLE_REPO_FIRST_COMMIT
@@ -432,9 +442,15 @@ def clone_with_force():
         third_repo_stat,
     ) = clone_with_force()
 
-    assert str(expected_path) == str(first_path), "Path should be the target location"
-    assert str(expected_path) == str(second_path), "Path should be the target location"
-    assert str(expected_path) == str(third_path), "Path should be the target location"
+    assert str(first_expected_path) == str(
+        first_path
+    ), "Path should be the target location"
+    assert str(first_expected_path) == str(
+        second_path
+    ), "Path should be the target location"
+    assert str(first_expected_path) == str(
+        third_path
+    ), "Path should be the target location"
 
     assert (
         first_repo_stat.st_mtime == second_repo_stat.st_mtime