Skip to content

4117433: Update _asset_utils.py to fix Performance issue #46572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sangeetha-cpu wants to merge 2 commits into from
+31 −9
Closed

4117433: Update _asset_utils.py to fix Performance issue #46572

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95d04d3
Update _asset_utils.py
sangeetha-cpu Apr 28, 2026
dbd96ba
Merge branch 'main' into main
lavakumarrepala Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 31 additions & 9 deletions sdk/ml/azure-ai-ml/azure/ai/ml/_utils/_asset_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
# pylint: disable=protected-access,too-many-lines

import hashlib
import json
import logging
import os
import json
import uuid
import warnings
from concurrent.futures import ThreadPoolExecutor, as_completed
Expand All @@ -16,7 +16,17 @@
from os import PathLike
from pathlib import Path
from platform import system
from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Optional, Tuple, Union, cast
from typing import (
TYPE_CHECKING,
Any,
Dict,
Iterable,
List,
Optional,
Tuple,
Union,
Comment on lines +20 to +27
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

There are trailing spaces in the multi-line from typing import (...) block (e.g., after TYPE_CHECKING, / Any, / Dict, etc.). These will trip linters (W291). Please remove the trailing whitespace and keep formatting consistent.

Suggested change
TYPE_CHECKING,
Any,
Dict,
Iterable,
List,
Optional,
Tuple,
Union,
TYPE_CHECKING,
Any,
Dict,
Iterable,
List,
Optional,
Tuple,
Union,

Copilot uses AI. Check for mistakes.
cast,
)

from colorama import Fore
from tqdm import TqdmWarning, tqdm
Expand Down Expand Up @@ -49,14 +59,19 @@
ModelContainersOperations,
ModelVersionsOperations,
)
from azure.ai.ml._restclient.arm_ml_service.models import (
DataVersionBase as DataVersionBaseData,
ModelVersion as ModelVersionData,
from azure.ai.ml._restclient.v2022_05_01.models import (
DataVersionBaseData,
ModelVersionData,
ModelVersionResourceArmPaginatedResult,
)
from azure.ai.ml._restclient.arm_ml_service.models import PendingUploadRequestDto
from azure.ai.ml._restclient.v2023_04_01.models import PendingUploadRequestDto
Comment on lines +62 to +67
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

The new restclient imports reference modules that don't exist in this package (azure.ai.ml._restclient.v2022_05_01.models and azure.ai.ml._restclient.v2023_04_01.models are not present under _restclient/). This will raise ImportError at runtime. Please switch these imports back to an existing restclient version (e.g., v2022_02_01_preview.models / v2023_04_01_preview.models) or keep using arm_ml_service.models, matching the operations you use in this file.

Copilot uses AI. Check for mistakes.
from azure.ai.ml._utils._pathspec import GitWildMatchPattern, normalize_file
from azure.ai.ml._utils.utils import convert_windows_path_to_unix, retry, snake_to_camel
from azure.ai.ml.constants._common import MAX_AUTOINCREMENT_ATTEMPTS, DefaultOpenEncoding, OrderString
from azure.ai.ml.constants._common import (
MAX_AUTOINCREMENT_ATTEMPTS,
DefaultOpenEncoding,
OrderString,
)
from azure.ai.ml.entities._assets.asset import Asset
from azure.ai.ml.exceptions import (
AssetPathException,
Expand Down Expand Up @@ -383,7 +398,14 @@ def get_upload_files_from_folder(
) -> List[str]:
path = Path(path)
upload_paths = []
for root, _, files in os.walk(path, followlinks=True):
for root, dirs, files in os.walk(path, followlinks=True, topdown=True):
# Skip ignored directories to avoid descending into them
# This significantly improves performance when large ignored folders (e.g., .venv) are present
dirs[:] = [
d for d in dirs
if not ignore_file.is_file_excluded(Path(root).joinpath(d).as_posix())
]
Comment on lines +401 to +407
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

The PR description is still the template text and doesn’t describe the actual change or link the relevant issue (the title suggests a performance fix). Please update the PR description to summarize what changed and reference the tracking issue/bug ID.

Copilot uses AI. Check for mistakes.

upload_paths += list(
traverse_directory(
root,
Expand Down Expand Up @@ -922,7 +944,7 @@ def _get_latest(
except StopIteration:
latest = None

if latest and isinstance(latest, ModelVersionData):
if latest and isinstance(latest, ModelVersionResourceArmPaginatedResult):
# Data list return object doesn't require this since its elements are already DatasetVersionResources
latest = cast(ModelVersionData, latest)
Comment on lines +947 to 949
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

The isinstance(latest, ModelVersionResourceArmPaginatedResult) branch is effectively dead: the ModelVersionsOperations.list() paging implementation yields elements from deserialized.value (typed as List[ModelVersionData]), so result.next() returns a ModelVersionData, not the paginated result wrapper. This check should likely be against ModelVersionData (or removed entirely) to keep the logic accurate and avoid confusion.

Suggested change
if latest and isinstance(latest, ModelVersionResourceArmPaginatedResult):
# Data list return object doesn't require this since its elements are already DatasetVersionResources
latest = cast(ModelVersionData, latest)

Copilot uses AI. Check for mistakes.
if not latest:
Expand Down
Loading