DYN-7145: handle large package upload #16375
Conversation
- refactored code to support async files and folders loading, moving the heavy UI-blocking operations to a sub thread
- changes to allow responsive frontend for cancellation and progress indication
- small null check for compatibilityMapList when loading Dynamo
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-7145
- revert accidental change to the reset function
- created tests for the new functionalities - aggregate warnings to prevent multiple prompts popping up during the async process - surface warnings/errors to the user once the loading is completed
There was a problem hiding this comment.
Pull Request Overview
This PR addresses DYN-7145 to improve Dynamo's handling of large package uploads by implementing asynchronous file processing and progress reporting. The change prevents Dynamo from hanging when processing packages with large numbers of files (>1GB was not actually the issue, but rather large file counts).
Key changes:
- Implemented asynchronous file processing with progress updates and cancellation support
- Added UI progress indicators and cancellation capabilities for large file operations
- Enhanced error handling and batch processing for better responsiveness
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
test/DynamoCoreWpf2Tests/PackageManager/PackageManagerUITests.cs |
Added comprehensive async tests and updated existing tests to use async methods |
src/DynamoPackages/PackageManagerClient.cs |
Added duplicate null check for compatibility map |
src/DynamoCoreWpf/Views/PackageManager/Components/PackageManagerWizard/PackageManagerWizard.xaml.cs |
Added upload progress and cancellation support to the wizard UI |
src/DynamoCoreWpf/ViewModels/PackageManager/PublishPackageViewModel.cs |
Implemented async file processing with progress reporting and cancellation |
src/DynamoCoreWpf/PublicAPI.Unshipped.txt |
Updated API surface for new async functionality |
src/DynamoCoreWpf/Properties/Resources.resx |
Added new localized error messages for multiple file failures |
src/DynamoCoreWpf/Properties/Resources.en-US.resx |
Added English translations for new error messages |
Files not reviewed (1)
- src/DynamoCoreWpf/Properties/Resources.Designer.cs: Language not supported
Comments suppressed due to low confidence (1)
test/DynamoCoreWpf2Tests/PackageManager/PackageManagerUITests.cs:1
- The removal of Thread.Sleep(500) may cause timing issues in UI tests. Consider adding a small wait or assertion that ensures the window is fully initialized before the assertion.
using System;
test/DynamoCoreWpf2Tests/PackageManager/PackageManagerUITests.cs
Outdated
Show resolved
Hide resolved
| UploadState = PackageUploadHandle.State.Uploading; | ||
| Uploading = true; |
There was a problem hiding this comment.
[nitpick] Consider consolidating these state changes into a single method to avoid potential inconsistencies. The same pattern appears multiple times throughout the file.
There was a problem hiding this comment.
I am bit hesitant to touch that logic - we use Uploading 19 times (references), and 33 for UploadState. This is legacy code and touches quite a few parts of our logic. If you think this needs rework, I would dedicate a separate PR for that @zeusongit
src/DynamoCoreWpf/ViewModels/PackageManager/PublishPackageViewModel.cs
Outdated
Show resolved
Hide resolved
- addressing misc comments from the team
|
1 test fails, but unrelated, merging: https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/pullRequestValidation/18579/ |
Purpose
Addresses https://jira.autodesk.com/browse/DYN-7145(https://jira.autodesk.com/browse/DYN-7145): As a Dynamo Dev, Dynamo should not hang and crash when the package upload is more than 1 GB.
Frontend PR - https://git.autodesk.com/Dynamo/PackagePublishWizard/pull/52
Analysis
The issue postulates that the PM cannot handle large packages - packages above 1Gb of data, and causes Dynamo to crash. Running a small amount of large files disproves this statement, as shown below. However, processing a large number of files is handled poorly at the moment, and without a UI update, causes the user to think Dynamo has crashed, although eventually the files will be processed.
> 1 Gb not a problem in itself
Introducing UI visual cue to show the progress of processing files as well as offloading this process from the main thread dramatically enhances the user experience.
Async process of file loading with progress update - processing Dynamo build with over 18k files
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
(FILL ME IN) Brief description of the fix / enhancement. Use N/A to indicate that the changes in this pull request do not apply to Release Notes. Mandatory section
Reviewers
@jasonstratton
@zeusongit
FYIs
@QilongTang