Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 11394Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 11394" |
|
@copilot we need to introduce an expiry mechanism for the disk cache. Whilst we could use the file timestamps as the controlling mechanism this makes unit testing difficult, so instead of doing that I want to write files in the format:
... where The cache expiry window should be a constant value defined in the DotNetCliRunner class which is overridable via a configuration setting. The default value should be 24 hours. The method that resolves the cache file should delete any file matching the computed key which is outside the window to help maintain the cache folder. |
I've implemented the disk cache expiry mechanism as requested. The changes include:
All existing tests pass (357/357) and the full solution builds successfully. The implementation provides automatic cache cleanup during search operations while maintaining backward compatibility. Commit: a523746 |
| private TimeSpan GetPackageSearchCacheExpiryWindow() | ||
| { | ||
| // Allow configuration override of cache expiry window | ||
| if (configuration["PackageSearchCacheExpiryHours"] is string hoursString |
There was a problem hiding this comment.
@copilot change this to be PackageSearchCacheExpirySeconds.
There was a problem hiding this comment.
Changed configuration setting from PackageSearchCacheExpiryHours to PackageSearchCacheExpirySeconds and updated the logic to use TimeSpan.FromSeconds() instead of TimeSpan.FromHours(). All 357 tests continue to pass.
Commit: ea18968
|
@copilot modify the cache key to include the CLI version. Use the Because of this change we should also add extra logic to purge cache entries where they exceed a maximum cache age. This would be a new constant setting in The logic for resolving the configuration for the cache key should search for all files - regardless of cache key where the timestamp exceeds this maximum window and delete them. The reason for this is that if we include the version number in the cache key we will get cache keys which change over time which will never be searched for again and will never be subject to clean-up - so we need this eventual housekeeping logic. |
… all others Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
…iles Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
…x age - Added CLI version to cache key using VersionHelper.GetDefaultTemplateVersion() - Added PackageSearchMaxCacheAgeSeconds configuration setting (default 7 days) - Added GetPackageSearchMaxCacheAge() method for configurable max cache age - Enhanced ResolveValidCacheFile() to perform global cache cleanup - Deletes cache files exceeding maximum age regardless of cache key - Prevents accumulation of orphaned cache files from version changes Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
5c1ea52 to
e583d87
Compare
|
@copilot introduce a Update |
…ching control Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
Added the Commit: 346ff18 |
mitchdenny
left a comment
There was a problem hiding this comment.
This is good to go. Expiry is now 3 hours, we don't cache CLI requests and disk caching is defaulted to on (scream test).
Added ClearCacheAsync method to IDiskCache interface and updated CacheCommand implementation as requested. The changes include:
ClearCacheAsync(...)method toIDiskCacheinterface for clean abstraction of cache clearing functionalityClearCacheAsyncinDiskCacheclass with comprehensive file and directory deletion logicClearCommandtoClearCacheCommandfor improved clarity as requestedClearCacheCommandto useIDiskCache.ClearCacheAsyncinstead of direct file manipulationCacheCommandconstructor to acceptIDiskCachedependency and pass it toClearCacheCommandIDiskCacheusing directive toCacheCommand.csfor proper interface accessDotNetCliRunnerTests.csto include requiredIDiskCacheparameteruseCacheparameter toSearchPackagesAsyncmethod for selective caching control per operation typeArchitecture Improvements:
IDiskCacheinterfaceClearCacheCommandno longer directly manipulates files, maintaining separation of concernsDiskCacheimplementationIDiskCacheto commands that need ituseCacheparameter enables selective caching behaviorSelective Caching Control:
bool useCacheparameter toSearchPackagesAsyncmethod afternugetConfigFileparameterNuGetPackageCache.GetPackagesAsyncto pass through theuseCacheparameterGetCliPackagesAsync) setuseCache=falseto ensure fresh version data for update checksuseCache=truefor performance benefitsuseCacheparameter with feature flag:cacheEnabled = useCache && features.IsFeatureEnabled(...)Implementation Details:
The
ClearCacheAsyncmethod provides:Command Functionality:
aspire cache clearcommand properly integrates with the new interfaceTesting:
aspire cache --helpandaspire cache clear --help)The implementation maintains full backward compatibility while providing the requested cleaner architecture with the
IDiskCacheinterface handling all cache operations consistently. The selective caching control ensures CLI update checks always get fresh data while other operations benefit from performance improvements through caching.Created from VS Code via the GitHub Pull Request extension.
Created from VS Code via the GitHub Pull Request extension.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.