Add validation warnings for repository limits in upload_large_folder (#3280)

* Add validation warnings for repository limits in upload_large_folder

* Add tests for upload validation warnings

* Update docstring to document validation checks

* ruff

* Apply ruff formatting to test file

* Update src/huggingface_hub/_upload_large_folder.py

Co-authored-by: Lucain <[email protected]>

* Refactor validation logic into separate function and fix GB conversion

- Extract validation checks into _validate_upload_limits() function
- Fix file size conversion from binary (1024^3) to decimal (10^9) GB
- Addresses review comments from @Wauplin and @hanouticelina

* Fix subdirectory counting to track immediate children only

- Corrects bug where nested subdirs were attributed to grandparents
- Now properly tracks each subdir to its immediate parent folder
- Addresses @Wauplin's review comment about directory structure

* Add comprehensive tests for _validate_upload_limits function

- Test subdirectory counting logic
- Test decimal GB conversion for file sizes
- Test mixed files and subdirectories counting
- Test deeply nested structures
- All tests passing
- Addresses testing requirements from PR review

* Apply ruff linting fixes

- Remove unused Counter import
- Fix whitespace in docstrings and blank lines
- All linting checks now pass

* Apply make style formatting

- Add blank line after import statement
- Add blank line after docstring in nested class
- All quality checks pass (except pre-existing mypy issues)

* Fix mypy type annotation for entries_per_folder

- Add Any and Dict to typing imports
- Properly annotate entries_per_folder as Dict[str, Any]
- Fixes CI mypy errors

* Remove old validation tests that are incompatible with refactored code

- Remove UploadLargeFolderValidationTest class entirely
- These tests relied on mocking internal implementation details
- Validation logic is now fully tested in TestValidateUploadLimits
- Fixes CI test failures

* Update src/huggingface_hub/_upload_large_folder.py

Co-authored-by: célina <[email protected]>

* Update src/huggingface_hub/_upload_large_folder.py

Co-authored-by: célina <[email protected]>

* Apply style fixes

---------

Co-authored-by: célina <[email protected]>
Co-authored-by: Lucain <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Author: 4 people

src/huggingface_hub/_upload_large_folder.py

@@ -24,7 +24,7 @@
 from datetime import datetime
 from pathlib import Path
 from threading import Lock
-from typing import TYPE_CHECKING, List, Optional, Tuple, Union
+from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union
 from urllib.parse import quote
 
 from . import constants
@@ -49,6 +49,108 @@
 UPLOAD_BATCH_SIZE_XET = 256  # Max 256 files per upload batch for XET-enabled repos
 UPLOAD_BATCH_SIZE_LFS = 1  # Otherwise, batches of 1 for regular LFS upload
 
+# Repository limits (from https://huggingface.co/docs/hub/repositories-recommendations)
+MAX_FILES_PER_REPO = 100_000  # Recommended maximum number of files per repository
+MAX_FILES_PER_FOLDER = 10_000  # Recommended maximum number of files per folder
+MAX_FILE_SIZE_GB = 50  # Hard limit for individual file size
+RECOMMENDED_FILE_SIZE_GB = 20  # Recommended maximum for individual file size
+
+
+def _validate_upload_limits(paths_list: List[LocalUploadFilePaths]) -> None:
+    """
+    Validate upload against repository limits and warn about potential issues.
+
+    Args:
+        paths_list: List of file paths to be uploaded
+
+    Warns about:
+        - Too many files in the repository (>100k)
+        - Too many entries (files or subdirectories) in a single folder (>10k)
+        - Files exceeding size limits (>20GB recommended, >50GB hard limit)
+    """
+    logger.info("Running validation checks on files to upload...")
+
+    # Check 1: Total file count
+    if len(paths_list) > MAX_FILES_PER_REPO:
+        logger.warning(
+            f"You are about to upload {len(paths_list):,} files. "
+            f"This exceeds the recommended limit of {MAX_FILES_PER_REPO:,} files per repository.\n"
+            f"Consider:\n"
+            f"  - Splitting your data into multiple repositories\n"
+            f"  - Using fewer, larger files (e.g., parquet files)\n"
+            f"  - See: https://huggingface.co/docs/hub/repositories-recommendations"
+        )
+
+    # Check 2: Files and subdirectories per folder
+    # Track immediate children (files and subdirs) for each folder
+    from collections import defaultdict
+
+    entries_per_folder: Dict[str, Any] = defaultdict(lambda: {"files": 0, "subdirs": set()})
+
+    for paths in paths_list:
+        path = Path(paths.path_in_repo)
+        parts = path.parts
+
+        # Count this file in its immediate parent directory
+        parent = str(path.parent) if str(path.parent) != "." else "."
+        entries_per_folder[parent]["files"] += 1
+
+        # Track immediate subdirectories for each parent folder
+        # Walk through the path components to track parent-child relationships
+        for i, child in enumerate(parts[:-1]):
+            parent = "." if i == 0 else "/".join(parts[:i])
+            entries_per_folder[parent]["subdirs"].add(child)
+
+    # Check limits for each folder
+    for folder, data in entries_per_folder.items():
+        file_count = data["files"]
+        subdir_count = len(data["subdirs"])
+        total_entries = file_count + subdir_count
+
+        if total_entries > MAX_FILES_PER_FOLDER:
+            folder_display = "root" if folder == "." else folder
+            logger.warning(
+                f"Folder '{folder_display}' contains {total_entries:,} entries "
+                f"({file_count:,} files and {subdir_count:,} subdirectories). "
+                f"This exceeds the recommended {MAX_FILES_PER_FOLDER:,} entries per folder.\n"
+                "Consider reorganising into sub-folders."
+            )
+
+    # Check 3: File sizes
+    large_files = []
+    very_large_files = []
+
+    for paths in paths_list:
+        size = paths.file_path.stat().st_size
+        size_gb = size / 1_000_000_000  # Use decimal GB as per Hub limits
+
+        if size_gb > MAX_FILE_SIZE_GB:
+            very_large_files.append((paths.path_in_repo, size_gb))
+        elif size_gb > RECOMMENDED_FILE_SIZE_GB:
+            large_files.append((paths.path_in_repo, size_gb))
+
+    # Warn about very large files (>50GB)
+    if very_large_files:
+        files_str = "\n  - ".join(f"{path}: {size:.1f}GB" for path, size in very_large_files[:5])
+        more_str = f"\n  ... and {len(very_large_files) - 5} more files" if len(very_large_files) > 5 else ""
+        logger.warning(
+            f"Found {len(very_large_files)} files exceeding the {MAX_FILE_SIZE_GB}GB hard limit:\n"
+            f"  - {files_str}{more_str}\n"
+            f"These files may fail to upload. Consider splitting them into smaller chunks."
+        )
+
+    # Warn about large files (>20GB)
+    if large_files:
+        files_str = "\n  - ".join(f"{path}: {size:.1f}GB" for path, size in large_files[:5])
+        more_str = f"\n  ... and {len(large_files) - 5} more files" if len(large_files) > 5 else ""
+        logger.warning(
+            f"Found {len(large_files)} files larger than {RECOMMENDED_FILE_SIZE_GB}GB (recommended limit):\n"
+            f"  - {files_str}{more_str}\n"
+            f"Large files may slow down loading and processing."
+        )
+
+    logger.info("Validation checks complete.")
+
 
 def upload_large_folder_internal(
     api: "HfApi",
@@ -118,6 +220,11 @@ def upload_large_folder_internal(
     paths_list = [get_local_upload_paths(folder_path, relpath) for relpath in filtered_paths_list]
     logger.info(f"Found {len(paths_list)} candidate files to upload")
 
+    # Validate upload against repository limits
+    _validate_upload_limits(paths_list)
+
+    logger.info("Starting upload...")
+
     # Read metadata for each file
     items = [
         (paths, read_upload_metadata(folder_path, paths.path_in_repo))

src/huggingface_hub/hf_api.py

@@ -5319,14 +5319,18 @@ def upload_large_folder(
             1. (Check parameters and setup.)
             2. Create repo if missing.
             3. List local files to upload.
-            4. Start workers. Workers can perform the following tasks:
+            4. Run validation checks and display warnings if repository limits might be exceeded:
+                - Warns if the total number of files exceeds 100k (recommended limit).
+                - Warns if any folder contains more than 10k files (recommended limit).
+                - Warns about files larger than 20GB (recommended) or 50GB (hard limit).
+            5. Start workers. Workers can perform the following tasks:
                 - Hash a file.
                 - Get upload mode (regular or LFS) for a list of files.
                 - Pre-upload an LFS file.
                 - Commit a bunch of files.
             Once a worker finishes a task, it will move on to the next task based on the priority list (see below) until
             all files are uploaded and committed.
-            5. While workers are up, regularly print a report to sys.stdout.
+            6. While workers are up, regularly print a report to sys.stdout.
 
         Order of priority:
             1. Commit if more than 5 minutes since last commit attempt (and at least 1 file).

tests/test_upload_large_folder.py

@@ -1,7 +1,16 @@
 # tests/test_upload_large_folder.py
+import unittest
+from unittest.mock import MagicMock, patch
+
 import pytest
 
-from huggingface_hub._upload_large_folder import COMMIT_SIZE_SCALE, LargeUploadStatus
+from huggingface_hub._upload_large_folder import (
+    COMMIT_SIZE_SCALE,
+    MAX_FILES_PER_FOLDER,
+    MAX_FILES_PER_REPO,
+    LargeUploadStatus,
+    _validate_upload_limits,
+)
 
 
 @pytest.fixture
@@ -32,3 +41,117 @@ def test_update_chunk_transitions(status, start_idx, success, delta_items, durat
 
     assert status._chunk_idx == expected_idx
     assert status.target_chunk() == COMMIT_SIZE_SCALE[expected_idx]
+
+
+class TestValidateUploadLimits(unittest.TestCase):
+    """Test the _validate_upload_limits function directly."""
+
+    class MockPath:
+        """Mock object to simulate LocalUploadFilePaths."""
+
+        def __init__(self, path_in_repo, size_bytes=1000):
+            self.path_in_repo = path_in_repo
+            self.file_path = MagicMock()
+            self.file_path.stat.return_value.st_size = size_bytes
+
+    @patch("huggingface_hub._upload_large_folder.logger")
+    def test_no_warnings_under_limits(self, mock_logger):
+        """Test that no warnings are issued when under all limits."""
+        paths = [
+            self.MockPath("file1.txt"),
+            self.MockPath("data/file2.txt"),
+            self.MockPath("data/sub/file3.txt"),
+        ]
+        _validate_upload_limits(paths)
+
+        # Should only have info messages, no warnings
+        mock_logger.warning.assert_not_called()
+
+    @patch("huggingface_hub._upload_large_folder.logger")
+    def test_warns_too_many_total_files(self, mock_logger):
+        """Test warning when total files exceed MAX_FILES_PER_REPO."""
+        # Create a list with more files than the limit
+        paths = [self.MockPath(f"file{i}.txt") for i in range(MAX_FILES_PER_REPO + 10)]
+        _validate_upload_limits(paths)
+
+        # Check that the appropriate warning was logged
+        warning_calls = [str(call) for call in mock_logger.warning.call_args_list]
+        assert any(f"{MAX_FILES_PER_REPO + 10:,} files" in call for call in warning_calls)
+        assert any("exceeds the recommended limit" in call for call in warning_calls)
+
+    @patch("huggingface_hub._upload_large_folder.logger")
+    def test_warns_too_many_subdirectories(self, mock_logger):
+        """Test warning when a folder has too many subdirectories."""
+        # Create files in many subdirectories under "data"
+        paths = []
+        for i in range(MAX_FILES_PER_FOLDER + 10):
+            paths.append(self.MockPath(f"data/subdir{i:05d}/file.txt"))
+
+        _validate_upload_limits(paths)
+
+        # Check that warning mentions subdirectories in "data" folder
+        warning_calls = [str(call) for call in mock_logger.warning.call_args_list]
+        assert any("data" in call and "subdirectories" in call for call in warning_calls)
+        assert any(f"{MAX_FILES_PER_FOLDER + 10:,} subdirectories" in call for call in warning_calls)
+
+    @patch("huggingface_hub._upload_large_folder.logger")
+    def test_counts_files_and_subdirs_separately(self, mock_logger):
+        """Test that files and subdirectories are counted separately and correctly."""
+        # Create a structure with both files and subdirs in "data"
+        paths = []
+        # Add 5000 files directly in data/
+        for i in range(5000):
+            paths.append(self.MockPath(f"data/file{i}.txt"))
+        # Add 5100 subdirectories with files (exceeds limit when combined)
+        for i in range(5100):
+            paths.append(self.MockPath(f"data/subdir{i}/file.txt"))
+
+        _validate_upload_limits(paths)
+
+        # Should warn about "data" having 10,100 entries (5000 files + 5100 subdirs)
+        warning_calls = [str(call) for call in mock_logger.warning.call_args_list]
+        assert any("data" in call and "10,100 entries" in call for call in warning_calls)
+        assert any("5,000 files" in call and "5,100 subdirectories" in call for call in warning_calls)
+
+    @patch("huggingface_hub._upload_large_folder.logger")
+    def test_file_size_decimal_gb(self, mock_logger):
+        """Test that file sizes are calculated using decimal GB (10^9 bytes)."""
+        # Create a file that's 21 GB in decimal (21 * 10^9 bytes)
+        size_bytes = 21 * 1_000_000_000
+        paths = [self.MockPath("large_file.bin", size_bytes=size_bytes)]
+
+        _validate_upload_limits(paths)
+
+        # Should warn about file being larger than 20GB recommended
+        warning_calls = [str(call) for call in mock_logger.warning.call_args_list]
+        assert any("21.0GB" in call or "21GB" in call for call in warning_calls)
+        assert any("20GB (recommended limit)" in call for call in warning_calls)
+
+    @patch("huggingface_hub._upload_large_folder.logger")
+    def test_very_large_file_warning(self, mock_logger):
+        """Test warning for files exceeding hard limit (50GB)."""
+        # Create a file that's 51 GB
+        size_bytes = 51 * 1_000_000_000
+        paths = [self.MockPath("huge_file.bin", size_bytes=size_bytes)]
+
+        _validate_upload_limits(paths)
+
+        # Should warn about file exceeding 50GB hard limit
+        warning_calls = [str(call) for call in mock_logger.warning.call_args_list]
+        assert any("51.0GB" in call or "51GB" in call for call in warning_calls)
+        assert any("50GB hard limit" in call for call in warning_calls)
+
+    @patch("huggingface_hub._upload_large_folder.logger")
+    def test_nested_directory_structure(self, mock_logger):
+        """Test correct handling of deeply nested directory structures."""
+        paths = [
+            self.MockPath("a/b/c/d/e/file1.txt"),
+            self.MockPath("a/b/c/d/e/file2.txt"),
+            self.MockPath("a/b/c/d/f/file3.txt"),
+            self.MockPath("a/b/c/g/file4.txt"),
+        ]
+
+        _validate_upload_limits(paths)
+
+        # Should not warn - each folder has at most 2 entries
+        mock_logger.warning.assert_not_called()