-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Load Balancing: Move temporary files and make them configurable to allow for media upload when load balancing the backoffice #20717
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a configurable location for temporary file uploads in Umbraco CMS. Previously, temporary file uploads were hardcoded to LocalTempPath + "/TemporaryFile", and this change allows users to customize this location via configuration settings.
- Adds
TemporaryFileUploadLocationconfiguration property toHostingSettings - Introduces
TemporaryFileUploadPathproperty toIHostingEnvironmentwith a default implementation - Refactors
LocalFileSystemTemporaryFileRepositoryto use the new configurable path
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Umbraco.Core/Configuration/Models/HostingSettings.cs | Adds new TemporaryFileUploadLocation configuration property |
| src/Umbraco.Core/Hosting/IHostingEnvironment.cs | Adds TemporaryFileUploadPath property with default implementation returning LocalTempPath |
| src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs | Implements TemporaryFileUploadPath using configuration with fallback to default path |
| src/Umbraco.Infrastructure/Persistence/Repositories/Implement/LocalFileSystemTemporaryFileRepository.cs | Refactored to use TemporaryFileUploadPath instead of inline path construction |
Co-authored-by: Copilot <[email protected]>
…ion-lb-friendly-and-configurable' into v17/hotfix/make-fileupload-location-lb-friendly-and-configurable
AndyButland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and works as expected. I've tested the outlined scenarios and with a default and custom folder I see the temporary files created in the expected place, and media uploads work with both configurations.
Problem
Previously, temporary files managed through
/umbraco/management/api/v1/temporary-filewere stored in_hostingEnvironment.LocalTempPath.In load-balanced setups where
Umbraco:CMS:Hosting:LocalTempStorageLocationis configured to"EnvironmentTemp", Umbraco ensures_hostingEnvironment.LocalTempPathis unique per instance. This is critical for Azure deployments where theUmbracofolder is shared between instances.However, this isolation creates a problem for temporary uploaded files, which need to be accessible across all instances. Consider this scenario:
/umbraco/management/api/v1/temporary-fileon server ASolution
Temporary files now always save to
umbraco/Data/TEMP/TemporaryFile/regardless of theUmbraco:CMS:Hosting:LocalTempStorageLocationsetting.For Azure: This works out of the box since the
umbracofolder is shared between instances when scaling out.For other setups (e.g., Docker): A new configuration option
Umbraco:CMS:Hosting:TemporaryFileUploadLocationallows you to specify a custom path (such as a shared drive or volume).Alternative approach: For advanced scenarios, you can implement a custom
ITemporaryFileRepositoryusing external storage like Azure Blob Storage (better suited for a package than core).Testing
Umbraco:CMS:Hosting:LocalTempStorageLocationto"EnvironmentTemp"and verify files uploaded via/umbraco/management/api/v1/temporary-fileare saved toumbraco/Data/TEMP/TemporaryFile/Umbraco:CMS:Hosting:TemporaryFileUploadLocationto a custom location and verify files are uploaded to that folder